fix BYTE return value #293
Conversation
| @@ -156,7 +156,7 @@ module.exports = { | |||
| return new BN(0) | |||
| } | |||
|
|
|||
| return word.shrn((31 - pos.toNumber()) * 8).andln(0xff) | |||
| return new BN(word.shrn((31 - pos.toNumber()) * 8).andln(0xff)) | |||
There was a problem hiding this comment.
it passes the relevant vm tests, fwiw:
ok 171 valid gas usage [file: byte10]
...
ok 297 valid gas usage [file: byte1]
ok 298 valid return value
There was a problem hiding this comment.
This PR passes it, master or both pass it?
It may be some combination of instructions which triggered this. @yann what was the actual issue you ran into?
There was a problem hiding this comment.
in remix we have to format the stack for each step to be compliant to the geth trace.
Before js vm would return a list of buffer for the stack , but it has been changed to BN.
And we end up by having a mix of BN and integger (due to BYTE)
There was a problem hiding this comment.
master (unpatched) passes the relevant VM tests. I haven't tried the PR.
There was a problem hiding this comment.
I see. I never thought about how return values from instructions are used afterward.
There was a problem hiding this comment.
I created an issue in tests repo ethereum/tests#448
There was a problem hiding this comment.
It would be nice if somehow we could include the hash of memory and the stack in the tests. That should catch a lot of things (but not all) tracing tries to catch.
There was a problem hiding this comment.
@yann300 the patch is correct and should be merged. All the stack items should be an instance of BN. Please report if you find any other discrepancy.
There was a problem hiding this comment.
@axic I'm not sure about the checking the stack contents. Implementations are allowed not to compute stack elements that are never used.
|
@axic @holgerd77 could this be merged and release? |
|
@jwasinger please rebase next time before merging to remove the extra merge commits 😉 |
to get the fix in ethereumjs/ethereumjs-monorepo#293 for #530
No description provided.