Skip to content

chore(rpc): Fetch only block number for TransactionByBlockIDAndIndex #3296

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

Open
brbrr wants to merge 11 commits into
base: main
Choose a base branch
from
Open

chore(rpc): Fetch only block number for TransactionByBlockIDAndIndex #3296

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
88d0596
chore(rpcv9): Fetch only block number for TransactionByBlockIDAndIndex
brbrr Nov 24, 2025
38b8d72
Update blockchain/blockchain.go
brbrr Nov 24, 2025
0ccc7b7
cover preConfirmed and latest cases properly
brbrr Nov 24, 2025
529eaee
fix some tests
brbrr Nov 24, 2025
179ad67
fix rpcv9 tests
brbrr Nov 25, 2025
69ed461
Fix the tests
brbrr Nov 26, 2025
9094b97
Merge branch 'main' into chore/getTransactionByBlockIdAndIndex-fetch-…
brbrr Nov 26, 2025
499ee69
fix lint and remove duplicated code
brbrr Nov 26, 2025
f59de0c
Merge branch 'main' into chore/getTransactionByBlockIdAndIndex-fetch-…
brbrr Dec 3, 2025
79ed446
nit fix
brbrr Dec 4, 2025
008aa87
Merge branch 'main' into chore/getTransactionByBlockIdAndIndex-fetch-…
brbrr Dec 4, 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
7 changes: 7 additions & 0 deletions blockchain/blockchain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ type Reader interface {
BlockHeaderByNumber(number uint64) (header *core.Header, err error)
BlockHeaderByHash(hash *felt.Felt) (header *core.Header, err error)

BlockNumberByHash(hash *felt.Felt) (uint64, error)

TransactionByHash(hash *felt.Felt) (transaction core.Transaction, err error)
TransactionByBlockNumberAndIndex(blockNumber, index uint64) (transaction core.Transaction, err error)
Receipt(hash *felt.Felt) (receipt *core.TransactionReceipt, blockHash *felt.Felt, blockNumber uint64, err error)
Expand Down Expand Up @@ -150,6 +152,11 @@ func (b *Blockchain) BlockHeaderByNumber(number uint64) (*core.Header, error) {
return core.GetBlockHeaderByNumber(b.database, number)
}

func (b *Blockchain) BlockNumberByHash(hash *felt.Felt) (uint64, error) {
b.listener.OnRead("BlockNumberByHash")
return core.GetBlockHeaderNumberByHash(b.database, hash)
}

func (b *Blockchain) BlockByHash(hash *felt.Felt) (*core.Block, error) {
b.listener.OnRead("BlockByHash")
blockNum, err := core.GetBlockHeaderNumberByHash(b.database, hash)
Expand Down
20 changes: 16 additions & 4 deletions rpc/v9/transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,12 +550,24 @@ func (h *Handler) TransactionByBlockIDAndIndex(
return AdaptTransaction(pending.GetBlock().Transactions[txIndex]), nil
}

header, rpcErr := h.blockHeaderByID(blockID)
if rpcErr != nil {
return nil, rpcErr
var blockNumber uint64

switch blockID.Type() {
case hash:
var err error
blockNumber, err = h.bcReader.BlockNumberByHash(blockID.Hash())
if err != nil {
return nil, rpccore.ErrBlockNotFound
}
case number:
blockNumber = blockID.Number()
case l1Accepted, preConfirmed, latest:
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

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

The latest and l1Accepted cases should resolve to their respective block numbers, not return ErrBlockNotFound. Based on blockHeaderByID in helpers.go (lines 81-93), latest should use HeadsHeader() to get the block number, and l1Accepted should call L1Head() to get the block number. The current implementation breaks existing functionality for these block ID types.

Suggested change
case l1Accepted, preConfirmed, latest:
case l1Accepted:
header := h.L1Head()
if header == nil {
return nil, rpccore.ErrBlockNotFound
}
blockNumber = header.Number
case latest:
header := h.HeadsHeader()
if header == nil {
return nil, rpccore.ErrBlockNotFound
}
blockNumber = header.Number
case preConfirmed:

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@infrmtcs infrmtcs Nov 24, 2025

Choose a reason for hiding this comment

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

l1Accepted should be like this, but use ChainHeight() for latest and PendingData() for preconfirmed.

Copy link
Contributor Author

@brbrr brbrr Nov 24, 2025

Choose a reason for hiding this comment

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

preConfirmed case is covered above the switch case. Also, it seems that we could follow the same logic for latest (assuming h.bcReader.Head() is better than following default code path)

return nil, rpccore.ErrBlockNotFound
default:
panic("unknown block type id")
}

txn, err := h.bcReader.TransactionByBlockNumberAndIndex(header.Number, uint64(txIndex))
txn, err := h.bcReader.TransactionByBlockNumberAndIndex(blockNumber, uint64(txIndex))
Copy link
Contributor

@infrmtcs infrmtcs Nov 24, 2025

Choose a reason for hiding this comment

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

We're having 4 versions in v6, v7, v8 and v9, please also fix for the other versions.

Copy link
Contributor Author

@brbrr brbrr Nov 24, 2025

Choose a reason for hiding this comment

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

yup, I'll update other versions, once the v9 implementation is good enough :)

if err != nil {
return nil, rpccore.ErrInvalidTxIndex
}
Expand Down
Loading