Skip to content

Conversation

@gumb0
Copy link
Member

@gumb0 gumb0 commented Dec 10, 2024

@codecov
Copy link

codecov bot commented Dec 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.31%. Comparing base (c8eb992) to head (702c007).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1084      +/-   ##
==========================================
+ Coverage   94.29%   94.31%   +0.02%     
==========================================
  Files         159      159              
  Lines       17160    17221      +61     
==========================================
+ Hits        16181    16242      +61     
  Misses        979      979              
Flag Coverage Δ
eof_execution_spec_tests 23.62% <43.03%> (+0.03%) ⬆️
ethereum_tests 26.63% <41.77%> (+0.04%) ⬆️
ethereum_tests_silkpre 18.94% <0.00%> (-0.08%) ⬇️
execution_spec_tests 20.95% <43.03%> (+0.06%) ⬆️
unittests 89.07% <92.40%> (+0.03%) ⬆️

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

Files with missing lines Coverage Δ
test/blockchaintest/blockchaintest_runner.cpp 78.23% <100.00%> (+0.14%) ⬆️
test/state/system_contracts.cpp 100.00% <100.00%> (ø)
test/state/test_state.cpp 100.00% <100.00%> (ø)
test/state/test_state.hpp 90.00% <ø> (ø)
test/t8n/t8n.cpp 88.09% <100.00%> (+0.07%) ⬆️
test/unittests/state_system_call_test.cpp 100.00% <100.00%> (ø)

@gumb0 gumb0 force-pushed the eip-7002-7251 branch 2 times, most recently from aeb0c13 to a76c241 Compare December 10, 2024 18:18
@gumb0 gumb0 self-assigned this Dec 11, 2024
@gumb0 gumb0 force-pushed the eip-7002-7251 branch 3 times, most recently from a35aeed to 805d987 Compare December 11, 2024 11:26
@gumb0 gumb0 force-pushed the eip-7002-7251 branch 3 times, most recently from 997baa7 to 792d126 Compare December 17, 2024 15:36
@gumb0 gumb0 marked this pull request as ready for review December 17, 2024 15:38
@gumb0 gumb0 requested review from chfast and pdobacz December 17, 2024 15:52
Copy link
Member

Choose a reason for hiding this comment

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

should we add more tests here for the new function (system_call_block_end)?

@gumb0 gumb0 marked this pull request as draft December 17, 2024 17:59
@gumb0 gumb0 marked this pull request as ready for review December 18, 2024 11:58
GetInputFn* get_input = nullptr; ///< How to get the input for the system call.
/// Type of requests returned by block end system call.
/// Ignored for block start system contracts.
Requests::Type request_type = Requests::Type::deposit;
Copy link
Member

Choose a reason for hiding this comment

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

This abstraction is quite weak because we have disjoint set of objects with get_input and requires_type. Fine for me for now though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree this is hacky

Copy link
Member

@chfast chfast left a comment

Choose a reason for hiding this comment

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

Maybe squash commits related to EIP-7251. They are tiny.


const Transaction empty_tx{};
Host host{rev, vm, state, block, block_hashes, empty_tx};
[[maybe_unused]] const auto res = vm.execute(host, rev, msg, code.data(), code.size());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[[maybe_unused]] const auto res = vm.execute(host, rev, msg, code.data(), code.size());
const auto res = vm.execute(host, rev, msg, code.data(), code.size());

@gumb0 gumb0 force-pushed the eip-7002-7251 branch 5 times, most recently from 178d4f8 to c2c75eb Compare December 19, 2024 11:15
@gumb0 gumb0 merged commit 50b095c into master Dec 19, 2024
25 checks passed
@gumb0 gumb0 deleted the eip-7002-7251 branch December 19, 2024 16:36
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.

4 participants