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

Move CALLF immediate argument validation to validate_instructions() #662

Merged
merged 2 commits into from
Jun 18, 2023

Conversation

gumb0
Copy link
Member

@gumb0 gumb0 commented Jun 5, 2023

Based on #663

Validating immediate argument is of CALLF shouldn't be part of stack validation, but rather instruction validation.

DATALOADN and RJUMPV (max_offset) arguments are already validated in validate_instructions() and #553 adds CREATE3 and RETURNCONTRACT there too, so it makes sense to have all these similar validations in one place.

@gumb0 gumb0 force-pushed the callf-instruction-validation branch from 618deb5 to 93cc5fb Compare June 6, 2023 10:43
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #662 (0de4808) into master (6c08a08) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 0de4808 differs from pull request most recent head 698c0ec. Consider uploading reports for the commit 698c0ec to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #662   +/-   ##
=======================================
  Coverage   97.38%   97.39%           
=======================================
  Files          80       80           
  Lines        7964     7973    +9     
=======================================
+ Hits         7756     7765    +9     
  Misses        208      208           
Flag Coverage Δ
blockchaintests 63.07% <0.00%> (-0.10%) ⬇️
statetests 74.00% <100.00%> (+0.02%) ⬆️
unittests 94.96% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
test/unittests/instructions_test.cpp 89.58% <ø> (ø)
lib/evmone/eof.cpp 83.42% <100.00%> (+0.13%) ⬆️
test/unittests/eof_validation_test.cpp 100.00% <100.00%> (ø)

@gumb0 gumb0 force-pushed the callf-instruction-validation branch from 0de4808 to 0a6f1d3 Compare June 6, 2023 16:53
@gumb0 gumb0 changed the base branch from master to dataloadn-truncated-imm June 6, 2023 16:54
@gumb0 gumb0 requested review from axic, chfast and rodiazet June 6, 2023 17:51
@gumb0 gumb0 added the EOF label Jun 6, 2023
Base automatically changed from dataloadn-truncated-imm to master June 18, 2023 20:16
@chfast chfast force-pushed the callf-instruction-validation branch from 0a6f1d3 to 698c0ec Compare June 18, 2023 20:16
@chfast chfast merged commit 1f8823e into master Jun 18, 2023
@chfast chfast deleted the callf-instruction-validation branch June 18, 2023 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants