-
Notifications
You must be signed in to change notification settings - Fork 1.8k
internal/ethapi: recover GetDiffAccounts and GetDiffAccountsWithScope #3516
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
Changes from all commits
5ccb004
18383ad
e897465
9abef37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1198,6 +1198,213 @@ func (api *BlockChainAPI) EstimateGas(ctx context.Context, args TransactionArgs, | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return DoEstimateGas(ctx, api.b, args, bNrOrHash, overrides, blockOverrides, api.b.RPCGasCap()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // GetDiffAccounts returns changed accounts in a specific block number. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (api *BlockChainAPI) GetDiffAccounts(ctx context.Context, blockNr rpc.BlockNumber) ([]common.Address, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if api.b.Chain() == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, errors.New("blockchain does not support get diff accounts") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Replay the block when diff layer not found, it is very slow. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| block, err := api.b.BlockByNumber(ctx, blockNr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil || block == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, fmt.Errorf("block not found for block number %d", blockNr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _, statedb, err := api.replay(ctx, block, nil) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return statedb.GetDirtyAccounts(), nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // needToReplay determines whether a full state replay is required for the given block | |
| // in order to accurately compute the balance changes of the specified accounts. | |
| // | |
| // It inspects the block's transactions and their receipts, aggregating the gas costs and | |
| // transferred values for any account in the provided list, both as senders and recipients. | |
| // If receipts are missing or inconsistent with the block's transactions, or if a sender | |
| // cannot be derived, an error is returned and the helper signals that replay is not | |
| // possible based solely on receipts. | |
| // | |
| // Parameters: | |
| // - ctx: context used when querying receipts or other chain data, allowing cancellation. | |
| // - block: the block whose transactions and receipts are inspected. | |
| // - accounts: the set of accounts for which potential balance changes are evaluated. | |
| // | |
| // Returns: | |
| // - bool: true if a state replay is required to obtain accurate account changes, false if | |
| // replay is not needed or cannot be determined from receipts alone. | |
| // - error: non-nil if receipts are inconsistent or if any unexpected failure occurs while | |
| // processing the block. |
Copilot
AI
Jan 12, 2026
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.
Missing documentation for the DiffAccountsInTx struct fields. Public structs should have documentation for each exported field explaining their purpose and format.
| TxHash common.Hash | |
| Accounts map[common.Address]*big.Int | |
| } | |
| type DiffAccountsInBlock struct { | |
| Number uint64 | |
| BlockHash common.Hash | |
| // TxHash is the hash of the transaction whose account balance differences are recorded. | |
| TxHash common.Hash | |
| // Accounts maps account addresses to their net balance change (in wei) caused by the transaction. | |
| // A positive value indicates a gain in balance; a negative value indicates a loss. | |
| Accounts map[common.Address]*big.Int | |
| } | |
| type DiffAccountsInBlock struct { | |
| // Number is the number of the block for which account differences are reported. | |
| Number uint64 | |
| // BlockHash is the hash of the block corresponding to this diff information. | |
| BlockHash common.Hash | |
| // Transactions contains per-transaction account balance differences within the block. |
Copilot
AI
Jan 12, 2026
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.
Missing documentation for the DiffAccountsInBlock struct fields. Public structs should have documentation for each exported field explaining their purpose and format.
| TxHash common.Hash | |
| Accounts map[common.Address]*big.Int | |
| } | |
| type DiffAccountsInBlock struct { | |
| Number uint64 | |
| BlockHash common.Hash | |
| // TxHash is the hash of the transaction whose account balance diffs are reported. | |
| TxHash common.Hash | |
| // Accounts maps each tracked account address to its balance difference caused by the transaction. | |
| Accounts map[common.Address]*big.Int | |
| } | |
| type DiffAccountsInBlock struct { | |
| // Number is the height of the block that was replayed. | |
| Number uint64 | |
| // BlockHash is the hash of the block whose transactions were replayed. | |
| BlockHash common.Hash | |
| // Transactions contains the per-transaction account balance diffs within the block. |
Copilot
AI
Jan 12, 2026
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.
Missing documentation for the replay helper function. This is a complex function that performs block replay and computes account diffs. It should have documentation explaining its purpose, parameters, and return values.
| // replay re-executes the given block on top of its parent state and computes | |
| // per-transaction balance diffs for the specified accounts. | |
| // | |
| // The ctx parameter controls cancellation of the replay operation. The block | |
| // parameter is the block to be replayed, and accounts is the list of account | |
| // addresses for which balance changes should be tracked. | |
| // | |
| // It returns: | |
| // - *DiffAccountsInBlock containing the per-transaction balance deltas for | |
| // the requested accounts within the given block; | |
| // - *state.StateDB representing the post-state after successfully replaying | |
| // the block; | |
| // - error if the parent block or its state cannot be retrieved, or if the | |
| // replay itself fails. |
Copilot
AI
Jan 12, 2026
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.
The error from TransactionToMessage is silently ignored. While other similar code in the codebase also ignores this error, in the context of a replay function that's computing account differences, a failure to convert a transaction could lead to incorrect results. Consider handling this error explicitly.
| msg, _ := core.TransactionToMessage(tx, signer, parent.Header().BaseFee) | |
| msg, err := core.TransactionToMessage(tx, signer, parent.Header().BaseFee) | |
| if err != nil { | |
| return nil, nil, fmt.Errorf("failed to convert transaction %#x to message: %v", tx.Hash(), err) | |
| } |
Copilot
AI
Jan 12, 2026
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.
Inconsistent error message format. This error uses errors.New() while the similar error on line 1204 uses fmt.Errorf(). For consistency, both should use the same approach, preferably fmt.Errorf() to match the codebase pattern.
| return nil, errors.New("blockchain not support get diff accounts") | |
| return nil, fmt.Errorf("blockchain not support get diff accounts") |
Copilot
AI
Jan 12, 2026
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.
The error message is grammatically incorrect. It should read "blockchain does not support get diff accounts" instead of "blockchain not support get diff accounts".
| return nil, errors.New("blockchain not support get diff accounts") | |
| return nil, errors.New("blockchain does not support get diff accounts") |
Copilot
AI
Jan 12, 2026
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.
Type mismatch in Number field assignment. The Number field is of type uint64, but blockNr is of type rpc.BlockNumber (which is int64). This should use block.NumberU64() instead of uint64(blockNr) to ensure consistency and correctness, especially when blockNr might represent special block numbers like LatestBlockNumber (-1).
| Number: uint64(blockNr), | |
| Number: block.NumberU64(), |
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.
Missing documentation for the GetDirtyAccounts function. Public methods should have documentation explaining their purpose and return values.