EIP 145 - Shift opcodes #43
Conversation
dziabko
changed the title
| @@ -166,6 +167,13 @@ type StateTest struct { | |||
| Post map[string][]stPostState `json:"post"` | |||
| } | |||
|
|
|||
| type ShiftTest struct { | |||
There was a problem hiding this comment.
This isn't used anywhere ?
|
Should rebase with development, operation function signatures do not match primarily and need to resolve conflicts |
| } | ||
|
|
||
| testTwoOperandOp(t, tests, opSAR, "sar") | ||
| } |
There was a problem hiding this comment.
Did it not work out cleanly to add in the tests from the testdata directory (from ETH as well) to include with these? These tests included should be sufficient, just curious
|
@dziabko please fix conflicts |
| @@ -273,7 +274,50 @@ func opMulmod(pc *uint64, env Environment, contract *Contract, memory *Memory, s | |||
| return nil, nil | |||
| } | |||
|
|
|||
| func opSha3(pc *uint64, env Environment, contract *Contract, memory *Memory, stack *stack) ([]byte, error) { | |||
| func opSHL(instr instruction, pc *uint64, env Environment, contract *Contract, memory *Memory, stack *stack) { | |||
There was a problem hiding this comment.
Build is failing because the function signature doesn't match the updated. Add the ([]byte, error) return to the signature and return those values respectively (nil returns for all changed functions including the opSha3 that was changed)
There was a problem hiding this comment.
why is instr instruction added as an argument?
There was a problem hiding this comment.
that's what it used to be, this is the old function signature
There was a problem hiding this comment.
it's also preventing build since he's passing these in as instrFn in the jump table, so either we remove that field or we add an additional field to instrFn and all the other opcodes
| @@ -22,6 +22,7 @@ import ( | |||
|
|
|||
| "github.com/eth-classic/go-ethereum/common" | |||
| "github.com/eth-classic/go-ethereum/crypto" | |||
| "github.com/ethereum/go-ethereum/common/math" | |||
|
I think we should PR this into a different branch, or move development into staging (for the testnet) so we dont confuse agharta changes with atlantis |
| } | ||
|
|
||
| func opSHR(instr instruction, pc *uint64, env Environment, contract *Contract, memory *Memory, stack *stack) { | ||
| shift, value := math.U256(stack.pop()), math.U256(stack.pop()) |
There was a problem hiding this comment.
remove references to math package
|
What's the status of this? |
|
Putting on ice so we can focus on Atlantis releases for now |
|
closing as stale |
Implemented EIP 145 (SAR, SHR, SHL opcodes). Implemented instruction_test.go file for basic tests. Added IsAghartha check. All tests pass for stShift test cases except for one specific test. Raised an issue for that test (#42 ).