Skip to content

Conversation

@brbrr
Copy link
Contributor

@brbrr brbrr commented Dec 2, 2025

Fixes: #3258

@brbrr brbrr self-assigned this Dec 2, 2025
Copilot AI review requested due to automatic review settings December 2, 2025 10:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the getTransactionReceipt RPC endpoint by reducing the number of database calls. Instead of making separate queries by transaction hash to get the transaction and receipt, it first retrieves the block number and index from the hash, then uses those coordinates to fetch the transaction and receipt directly. This reduces database lookups from potentially 3+ calls down to 3 calls (one for the index lookup, one for transaction, one for receipt).

Key changes:

  • Introduced new methods TxBlockNumberAndIndexByHash and ReceiptByBlockNumberAndIndex to enable block-coordinate-based lookups
  • Refactored TransactionReceiptByHash to use the new lookup pattern

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
rpc/v9/transaction.go Updated TransactionReceiptByHash to use new block-coordinate-based lookup methods
blockchain/blockchain.go Added TxBlockNumberAndIndexByHash and ReceiptByBlockNumberAndIndex methods to Reader interface
core/accessors.go Added GetReceiptByBlockNumIndex helper function for direct receipt lookup by coordinates
mocks/mock_blockchain.go Generated mock implementations for new Reader interface methods
rpc/v9/transaction_test.go Updated all test expectations to use new method call patterns
rpc/v9/subscriptions_test.go Updated subscription test expectations for new method calls
rpc/v9/l1_test.go Updated L1 message status test expectations and fixed type inconsistencies

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Co-authored-by: Copilot <[email protected]>
Signed-off-by: Yaroslav Kukharuk <[email protected]>
brbrr and others added 2 commits December 2, 2025 12:43
Co-authored-by: Dat Duong <[email protected]>
Signed-off-by: Yaroslav Kukharuk <[email protected]>
@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

❌ Patch coverage is 32.65306% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.07%. Comparing base (584e500) to head (d2ce1c8).

Files with missing lines Patch % Lines
blockchain/blockchain.go 0.00% 13 Missing ⚠️
core/accessors.go 0.00% 8 Missing ⚠️
rpc/v8/transaction.go 57.14% 4 Missing and 2 partials ⚠️
rpc/v10/transactions.go 42.85% 3 Missing and 1 partial ⚠️
rpc/v9/transaction.go 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3320      +/-   ##
==========================================
- Coverage   76.20%   76.07%   -0.13%     
==========================================
  Files         346      346              
  Lines       32690    32722      +32     
==========================================
- Hits        24912    24894      -18     
- Misses       5987     6029      +42     
- Partials     1791     1799       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@rodrigo-pino rodrigo-pino left a comment

Choose a reason for hiding this comment

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

Overview review, I am not sure I notice how we are doing less calls here

Comment on lines +36 to +37
BlockNumberAndIndexByTxHash(
hash *felt.TransactionHash) (blockNumber uint64, index uint64, err error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's improve the formatting style here. The standard we are trying to have in Juno looks like:

BlockNumberAndIndexByTxHash(
    hash *felt.TransactionHash,
) (blockNumber uint64, index uint64, err error)

Comment on lines +40 to +42
ReceiptByBlockNumberAndIndex(
blockNumber, index uint64) (receipt *core.TransactionReceipt, blockHash *felt.Felt, err error)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar here

ReceiptByBlockNumberAndIndex( 
	blockNumber, index uint64,
) (receipt *core.TransactionReceipt, blockHash *felt.Felt, err error)

Comment on lines +268 to +276
) (*TransactionReceipt, error) {
var numIdxKey db.BlockNumIndexKey
numIdxKey.Number = num
numIdxKey.Index = index
receipt, err := ReceiptsByBlockNumberAndIndexBucket.Get(r, numIdxKey)
if err != nil {
return nil, err
}
return &receipt, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please return TransactionReceipt by value

@brbrr
Copy link
Contributor Author

brbrr commented Dec 5, 2025

Overview review, I am not sure I notice how we are doing less calls here

Based on my quick look, TransactionByHash reads from:

  • TransactionBlockNumbersAndIndicesByHashBucket
  • TransactionsByBlockNumberAndIndexBucket

Receipt reads from:

  • TransactionBlockNumbersAndIndicesByHashBucket
  • TransactionBlockNumbersAndIndicesByHashBucket
  • ReceiptsByBlockNumberAndIndexBucket
  • BlockHeadersByNumber

with this PR, non-pending request will read from:

  • TransactionBlockNumbersAndIndicesByHashBucket
  • TransactionsByBlockNumberAndIndexBucket
  • ReceiptsByBlockNumberAndIndexBucket
  • BlockHeadersByNumber

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.

starknet_getTransactionReceipt queries TransactionBlockNumbersAndIndicesByHash bucket twice

4 participants