Skip to content

ethclient: fix retrieval of pending block #31504

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 2, 2025
Merged

ethclient: fix retrieval of pending block #31504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ad5d463
ethclient: change Hash field of rpcBlock to pointer
islishude Mar 27, 2025
8a84834
ethclient: add test for pending block retrieval
islishude Mar 27, 2025
fc341f4
ethclient: use pending block hash to key tx sender cache
lightclient May 1, 2025
0d10a01
ethclient: fixup comment
lightclient May 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions ethclient/ethclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (ec *Client) BlockReceipts(ctx context.Context, blockNrOrHash rpc.BlockNumb
}

type rpcBlock struct {
Hash common.Hash `json:"hash"`
Hash *common.Hash `json:"hash"`
Transactions []rpcTransaction `json:"transactions"`
UncleHashes []common.Hash `json:"uncles"`
Withdrawals []*types.Withdrawal `json:"withdrawals,omitempty"`
Expand All @@ -158,6 +158,12 @@ func (ec *Client) getBlock(ctx context.Context, method string, args ...interface
if err := json.Unmarshal(raw, &body); err != nil {
return nil, err
}
// Pending blocks don't return a block hash, compute it for sender caching.
if body.Hash == nil {
tmp := head.Hash()
body.Hash = &tmp
}

Comment on lines +161 to +166
Copy link
Member

@lightclient lightclient May 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Mostly added this addition here. This allows us to correctly set the sender caching. It seems like a nice thing to have, since we're returning a types.Block anyways the user can compute the block hash and it's possible they later call TransactionSender with it. Felt weird to just drop that optimization on the ground for pending blocks.

// Quick-verify transaction and uncle lists. This mostly helps with debugging the server.
if head.UncleHash == types.EmptyUncleHash && len(body.UncleHashes) > 0 {
return nil, errors.New("server returned non-empty uncle list but block header indicates no uncles")
Expand Down Expand Up @@ -199,7 +205,7 @@ func (ec *Client) getBlock(ctx context.Context, method string, args ...interface
txs := make([]*types.Transaction, len(body.Transactions))
for i, tx := range body.Transactions {
if tx.From != nil {
setSenderFromServer(tx.tx, *tx.From, body.Hash)
setSenderFromServer(tx.tx, *tx.From, *body.Hash)
}
txs[i] = tx.tx
}
Expand Down
33 changes: 29 additions & 4 deletions ethclient/ethclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,12 @@ func testTransactionInBlock(t *testing.T, client *rpc.Client) {
if tx.Hash() != testTx2.Hash() {
t.Fatalf("unexpected transaction: %v", tx)
}

// Test pending block
_, err = ec.BlockByNumber(context.Background(), big.NewInt(int64(rpc.PendingBlockNumber)))
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
}

func testChainID(t *testing.T, client *rpc.Client) {
Expand Down Expand Up @@ -619,6 +625,21 @@ func testAtFunctions(t *testing.T, client *rpc.Client) {
if gas != 21000 {
t.Fatalf("unexpected gas limit: %v", gas)
}

// Verify that sender address of pending transaction is saved in cache.
pendingBlock, err := ec.BlockByNumber(context.Background(), big.NewInt(int64(rpc.PendingBlockNumber)))
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
// No additional RPC should be required, ensure the server is not asked by
// canceling the context.
sender, err := ec.TransactionSender(newCanceledContext(), pendingBlock.Transactions()[0], pendingBlock.Hash(), 0)
if err != nil {
t.Fatal("unable to recover sender:", err)
}
if sender != testAddr {
t.Fatal("wrong sender:", sender)
}
}

func testTransactionSender(t *testing.T, client *rpc.Client) {
Expand All @@ -640,10 +661,7 @@ func testTransactionSender(t *testing.T, client *rpc.Client) {

// The sender address is cached in tx1, so no additional RPC should be required in
// TransactionSender. Ensure the server is not asked by canceling the context here.
canceledCtx, cancel := context.WithCancel(context.Background())
cancel()
<-canceledCtx.Done() // Ensure the close of the Done channel
sender1, err := ec.TransactionSender(canceledCtx, tx1, block2.Hash(), 0)
sender1, err := ec.TransactionSender(newCanceledContext(), tx1, block2.Hash(), 0)
if err != nil {
t.Fatal(err)
}
Expand All @@ -662,6 +680,13 @@ func testTransactionSender(t *testing.T, client *rpc.Client) {
}
}

func newCanceledContext() context.Context {
ctx, cancel := context.WithCancel(context.Background())
cancel()
<-ctx.Done() // Ensure the close of the Done channel
return ctx
}

func sendTransaction(ec *ethclient.Client) error {
chainID, err := ec.ChainID(context.Background())
if err != nil {
Expand Down