[PAN-2903] Implement EIP-1344 ChainID Opcode #1690
Conversation
Add the ChainID Opcode
|
Force push was a revert, EIP1108 was accidentally pushed on this branch. |
| @@ -285,12 +286,13 @@ private MainnetProtocolSpecs() {} | |||
| final OptionalInt configContractSizeLimit, | |||
| final OptionalInt configStackSizeLimit, | |||
| final boolean enableRevertReason) { | |||
| checkArgument(chainId.isPresent(), "Istanbul requires the use of chainId"); | |||
There was a problem hiding this comment.
If you trace back to where the chainId is set, it looks like it's only ever missing in test cases. Might be worth going through and making the chainId argument a plain unwrapped BigInteger, in which case we wouldn't need this check.
There was a problem hiding this comment.
FixedDifficultyProtocolSchedule uses it too. It's at least a 4 file change, larger than this PR, so that level of change would deserve its own PR.
There was a problem hiding this comment.
There's no requirement that a chain has a chainId - it may be omitted from the genesis config. This is a slightly interesting hole in the ChainID opcode spec. Doesn't apply to any public chain but there may be chains out there it does affect. So maybe ChainID opcode should return 0 when there is no chain ID?
| } | ||
|
|
||
| @Test | ||
| public void shouldReturnChanId() { |
There was a problem hiding this comment.
| public void shouldReturnChanId() { | |
| public void shouldReturnChainId() { |
| public void shouldReturnChanId() { | ||
| final ArgumentCaptor<Bytes32> arg = ArgumentCaptor.forClass(Bytes32.class); | ||
| operation.execute(messageFrame); | ||
| Mockito.verify(messageFrame).pushStackItem(arg.capture()); |
There was a problem hiding this comment.
You should probably also verify that pushStackItem is only called once.
PR description
Add the ChainID Opcode