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.

@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

❌ Patch coverage is 34.69388% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.16%. Comparing base (a50928a) to head (8c1c865).

Files with missing lines Patch % Lines
blockchain/blockchain.go 0.00% 13 Missing ⚠️
rpc/v8/transaction.go 60.00% 4 Missing and 2 partials ⚠️
core/accessors.go 0.00% 5 Missing ⚠️
rpc/v10/transactions.go 50.00% 3 Missing and 1 partial ⚠️
rpc/v9/transaction.go 50.00% 3 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (34.69%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3320      +/-   ##
==========================================
- Coverage   76.24%   76.16%   -0.08%     
==========================================
  Files         349      349              
  Lines       33092    33121      +29     
==========================================
- Hits        25231    25228       -3     
- Misses       6063     6087      +24     
- Partials     1798     1806       +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

@rodrodros rodrodros 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

@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

update v9 and tests

Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Yaroslav Kukharuk <i.kukharuk@gmail.com>

Apply suggestion from @infrmtcs

Co-authored-by: Dat Duong <DuongDat.Informatics@rocketmail.com>
Signed-off-by: Yaroslav Kukharuk <i.kukharuk@gmail.com>

update new method name

use felt.TransactionHash

v8 transaction and tests

fix linting errors

apply the change to v10

update styling to be inline with codebase

address feedback

small fix

Apply suggestions from code review

Co-authored-by: Dat Duong <DuongDat.Informatics@rocketmail.com>
Signed-off-by: Yaroslav Kukharuk <i.kukharuk@gmail.com>

return receipt by value and update the tests

style
@rodrodros rodrodros force-pushed the update/reduce-db-calls-getTransactionReceipt branch from 67d8fe5 to 8c1c865 Compare January 2, 2026 15:39
@rodrodros rodrodros deployed to Development January 2, 2026 15:46 — with GitHub Actions Active
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