Transpiling Hardcoded Constructor Values #169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
K-Ho
approved these changes
Jun 30, 2020
| buildJSON: any, | ||
| isDeployedBytecode: boolean | ||
| ): Buffer => { | ||
| const auxDataObject = buildJSON.evm.legacyAssembly['.data'] |
There was a problem hiding this comment.
just double checking that this should be ['.data'] and not .data or ['data']
There was a problem hiding this comment.
Yep! Weird I know.
Comment on lines
+59
to
+63
| bytecodeWithoutAuxdata.fill( | ||
| 0, | ||
| auxDataPosition, | ||
| auxDataPosition + auxDataBuf.byteLength | ||
| ) |
packages/rollup-dev-tools/test/transpiler/abi-encoded-constants-transpilation.spec.ts
Outdated
Show resolved
Hide resolved
packages/rollup-dev-tools/test/transpiler/abi-encoded-constants-transpilation.spec.ts
Outdated
Show resolved
Hide resolved
Comment on lines
99
to
123
| const expectedInnerBytesBeingHashed = Buffer.from( | ||
| ethers.utils.toUtf8Bytes( | ||
| 'EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)' | ||
| ) | ||
| ) | ||
| log.debug( | ||
| `expectedInnerBytesBeingHashed: ${bufToHexString( | ||
| expectedInnerBytesBeingHashed | ||
| )}` | ||
| ) | ||
| const expectedInnerHashRaw = ethers.utils.keccak256( | ||
| expectedInnerBytesBeingHashed | ||
| ) | ||
| log.debug(`expectedInnerHashRaw: ${expectedInnerHashRaw}`) | ||
| const expectedInnerHashAndValEncoded = ethers.utils.defaultAbiCoder.encode( | ||
| ['bytes32', 'uint256'], | ||
| [expectedInnerHashRaw, 1] | ||
| ) | ||
| log.debug( | ||
| `expectedInnerHashAndValEncoded: ${expectedInnerHashAndValEncoded}` | ||
| ) | ||
| const expectedOuterHash = ethers.utils.keccak256( | ||
| expectedInnerHashAndValEncoded | ||
| ) | ||
| log.debug(`expected final outer hash: ${expectedOuterHash}`) |
There was a problem hiding this comment.
might be cleaner to just do this all in one go and not log the intermediate values (e.g. https://github.com/ethereum-optimism/uniswap-v2-core/blob/706c0ef8749511d9c36a05e459a2ace9099c85ce/test/UniswapV2ERC20.spec.ts#L37-L52)
There was a problem hiding this comment.
Ah yeah, this was set up for debugging to figure out which stage it was failing at. Removing
…s-transpilation.spec.ts Co-authored-by: Kevin Ho <[email protected]>
snario
pushed a commit
that referenced
this pull request
Apr 14, 2021
bnoieh
pushed a commit
to bnoieh/optimism
that referenced
this pull request
Apr 3, 2024
ClaytonNorthey92
added a commit
to hemilabs/optimism
that referenced
this pull request
Dec 9, 2024
c9dbc5f58 Test fix & fmt fcf735497 ExternalHeaderMode rebase f09d4e5ff Add for wind/unwind support to tbc (ethereum-optimism#159) 5c764a0cd popm: exclude tx outputs that would be dust (ethereum-optimism#186) 1f168f225 popm/wasm: fix dispatch params for {add,remove}EventListener (ethereum-optimism#181) 383611feb popm/wasm: add 'minerStatus' method (ethereum-optimism#178) 6a94b4c0b popm/wasm: rename KeyResult fields and add bitcoinScriptHash (ethereum-optimism#177) 9c4cea583 popm/wasm: add events and clean up globals (ethereum-optimism#175) a2372394d popm: improve UTXO selection when creating Bitcoin transaction (ethereum-optimism#173) 939813cac popm/wasm: fix ethereum address when generating and parsing keys (ethereum-optimism#174) 9245995ef Updated localnet to use more recent commits of optimism and op-geth (ethereum-optimism#139) 159fbc5f5 ci: improve runtime of high usage CI workflows (ethereum-optimism#163) 60a3489db popm/wasm: add 'bitcoinAddressToScriptHash' method (ethereum-optimism#169) fe9752a5a popm/wasm: add 'parseKey' method (ethereum-optimism#168) 65aa0caef ci: add registry-url to setup-node in release (ethereum-optimism#167) 8922ce805 popm/wasm: add new @hemilabs/pop-miner NPM package (ethereum-optimism#162) 67166046b scripts: add release script (ethereum-optimism#164) eea96059b ci: add labeler action (ethereum-optimism#165) git-subtree-dir: heminetwork git-subtree-split: c9dbc5f58a7f997fa4b3af0d765a2967ed3462d1
blockchaindevsh
added a commit
to blockchaindevsh/optimism
that referenced
this pull request
Jan 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
It turned out that if a constructor looks something like
Then the hardcoded val is handled a little differently than constants--it's put after the auxdata in the initcode. This introduced a bug where we accidentally purged that hardcoded value with
stripAuxData().This PR fixes by removing auxdata only if there's no additional data proceeding it. If there is, then it instead fills the auxdata with
00s.Questions
N/A
Metadata
Fixes
Contributing Agreement