-
Notifications
You must be signed in to change notification settings - Fork 104
feat: remove metadata receipts and account balances #300
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
Conversation
✅ Heimdall Review Status
|
refcell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking nit: maybe add a test
1f45a56 to
362c85f
Compare
Approved review 3631920391 from refcell is now dismissed due to new commit. Re-request for approval.
bab581f to
b50a2c8
Compare
|
Instead of making these optional, can we just remove the account balance/receipts completely? These are a holdover from the time when we didn't execute Flashblocks, but rather just stored them in a cache. |
|
I think most of the failing tests are due to relying on passed in receipts rather than actually executing. I'll need to update the EVM bytecode to actually emit logs instead of assuming the receipts are correct. |
|
Updated the PR to remove account balances as well (these should still serialize fine if they exist due to not using strict mode serde). Also, updated tests to actually emit logs instead of depending on receipts/balances passed in from the flashblock. |
refcell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
|
Review Error for refcell @ 2026-01-07 16:56:18 UTC |
refcell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops forgot mfa
The metadata field used to contain receipts and changed account balances as a performance optimization to be able to serve RPC requests without actually executing the block, but we still need to execute the block in most cases anyway. This removes the receipts and changed_account_balances fields from the response. These are generated during flashblock execution instead. This still preserves skipping execution of transactions that were executed in previous flashblocks as long as we have the receipt and state from the previous execution. In either the cached or executed path, cumulative_gas and next_log_index are updated. The tests also needed to be fixed since they relied on the receipts for logs and the changed balances. Now, since the transactions are actually executed, we need to actually emit logs from the contracts when executed and send balance to the address rather than relying on changed_account_balances.
d040dab to
5d35253
Compare
|
resolved branch conflicts ^ |
The metadata field used to contain receipts and changed account balances as a performance optimization to be able to serve RPC requests without actually executing the block, but we still need to execute the block in most cases anyway. This removes the
receiptsandchanged_account_balancesfields from the response.These are generated during flashblock execution instead.
This still preserves skipping execution of transactions that were executed in previous flashblocks as long as we have the receipt and state from the previous execution. In either the cached or executed path,
cumulative_gasandnext_log_indexare updated.The tests also needed to be fixed since they relied on the receipts for logs and the changed balances. Now, since the transactions are actually executed, we need to actually emit logs from the contracts when executed and send balance to the address rather than relying on
changed_account_balances.Fixes CHAIN-2657