Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EVM: Improve the call tests #1043

Merged
merged 1 commit into from
Jan 13, 2023
Merged

EVM: Improve the call tests #1043

merged 1 commit into from
Jan 13, 2023

Conversation

arajasek
Copy link
Contributor

  • Don't use the same methodNum as expected return data to avoid confusion
  • Include the success/failure of the call in the return data
  • Take methodNum and expected success as parameters for more extensible tests

It would likely be worth applying these improvements to the other tests in the file too, but we're sprinting right now.

Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the off by 1s?

push1 0x00
push1 0x00
push1 0x00 # offset
push1 0x01 # dest offset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first byte is the exit code pushed put into memory

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^what melanie said -- i can add comments making clear.

# return
returndatasize
returndatasize
push1 0x01
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

Comment on lines +406 to +409
# write exit code memory
push1 0x00 # offset
mstore8

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vyzo here

actors/evm/tests/call.rs Outdated Show resolved Hide resolved
* Don't use the same methodNum as expected return data to avoid confusion
* Include the success/failure of the call in the return data
* Take methodNum and expected success as parameters for more extensible tests
@arajasek arajasek enabled auto-merge (squash) January 13, 2023 16:40
@arajasek arajasek merged commit a50a77c into next Jan 13, 2023
@arajasek arajasek deleted the asr/call-tests branch January 13, 2023 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants