Skip to content

Conversation

@brbrr
Copy link
Contributor

@brbrr brbrr commented Nov 24, 2025

fixes #3257

@brbrr brbrr self-assigned this Nov 24, 2025
Copilot AI review requested due to automatic review settings November 24, 2025 10:26
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 TransactionByBlockIDAndIndex method by fetching only the block number instead of the full block header. It introduces a new BlockNumberByHash method in the blockchain reader interface to support this optimization. However, the implementation has critical bugs that break existing functionality for latest and l1Accepted block ID types.

  • Adds BlockNumberByHash method to blockchain reader interface for efficient block number lookup
  • Refactors TransactionByBlockIDAndIndex to avoid fetching full headers for hash-based lookups
  • Introduces bugs for latest and l1Accepted block ID types

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
rpc/v9/transaction.go Refactors TransactionByBlockIDAndIndex to use block number directly instead of fetching full header, but incorrectly returns ErrBlockNotFound for latest and l1Accepted block types
blockchain/blockchain.go Adds BlockNumberByHash interface method and implementation to retrieve block number from hash without fetching full header

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

@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

❌ Patch coverage is 88.15789% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.19%. Comparing base (a4a60ba) to head (e0e9636).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
rpc/v9/transaction.go 83.33% 3 Missing and 1 partial ⚠️
blockchain/blockchain.go 0.00% 3 Missing ⚠️
rpc/v8/transaction.go 88.88% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3296      +/-   ##
==========================================
- Coverage   76.20%   76.19%   -0.01%     
==========================================
  Files         350      350              
  Lines       33177    33232      +55     
==========================================
+ Hits        25281    25320      +39     
- Misses       6088     6105      +17     
+ Partials     1808     1807       -1     

☔ 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.

@brbrr brbrr changed the title chore(rpcv9): Fetch only block number for TransactionByBlockIDAndIndex chore(rpc): Fetch only block number for TransactionByBlockIDAndIndex Nov 26, 2025
@rodrodros rodrodros force-pushed the chore/getTransactionByBlockIdAndIndex-fetch-only-block-number branch from 008aa87 to 7c449a5 Compare January 2, 2026 14:59
@rodrodros rodrodros force-pushed the chore/getTransactionByBlockIdAndIndex-fetch-only-block-number branch from 7c449a5 to 85c4b99 Compare January 2, 2026 15:00
@brbrr brbrr merged commit 10eb1fd into main Jan 7, 2026
19 of 22 checks passed
@brbrr brbrr deleted the chore/getTransactionByBlockIdAndIndex-fetch-only-block-number branch January 7, 2026 08:47
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_getTransactionByBlockIdAndIndex doesn't have to query block header

5 participants