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

Rename error non_empty_stack_on_terminating_instruction #727

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

hugo-dc
Copy link
Member

@hugo-dc hugo-dc commented Nov 9, 2023

Error non_empty_stack_on_terminating_instruction is currently only used when there is a mismatch between the number of stack items in current function context and the total number of defined outputs, which is no longer accurate.

This PR changes the error name to indicate outputs mismatch.

@hugo-dc hugo-dc requested review from gumb0 and rodiazet and removed request for rodiazet November 9, 2023 06:18
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #727 (056999c) into master (754fc9e) will not change coverage.
The diff coverage is 71.42%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #727   +/-   ##
=======================================
  Coverage   97.84%   97.84%           
=======================================
  Files         109      109           
  Lines       10318    10318           
=======================================
  Hits        10096    10096           
  Misses        222      222           
Flag Coverage Δ
blockchaintests 60.47% <0.00%> (ø)
statetests 61.79% <33.33%> (ø)
statetests-silkpre 26.23% <0.00%> (ø)
unittests 95.91% <71.42%> (ø)

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

Files Coverage Δ
lib/evmone/eof.hpp 100.00% <ø> (ø)
test/unittests/eof_validation_test.cpp 100.00% <100.00%> (ø)
lib/evmone/eof.cpp 83.57% <50.00%> (ø)

@gumb0
Copy link
Member

gumb0 commented Nov 9, 2023

Also please change commit message to Rename error non_empty_stack_on_terminating_instruction (we keep the first line capitalized as title and short so that github does not cut it in UI)

@hugo-dc hugo-dc force-pushed the retf-outputs branch 2 times, most recently from a70a284 to 53a1b87 Compare November 23, 2023 01:37
@gumb0
Copy link
Member

gumb0 commented Nov 24, 2023

Looks good, but can you leave only RETF change in this PR, then we will merge it, and then I'll rebase JUMPF?

@gumb0
Copy link
Member

gumb0 commented Nov 28, 2023

Looks good, but can you leave only RETF change in this PR, then we will merge it, and then I'll rebase JUMPF?

Now JUMPF is merged, you can just rebase this PR instead.

@gumb0 gumb0 marked this pull request as ready for review December 5, 2023 20:20
@gumb0 gumb0 merged commit e840e6f into ethereum:master Dec 5, 2023
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.

2 participants