SHARD-2024: Fix incorrect transaction index

[jintukumardas]

User description

Correct the incorrect transactionIndex output for eth_getTransactionByBlockHashAndIndex and eth_getTransactionByBlockNumberAndIndex.

---

PR Type

Bug fix

---

Description

• Fix incorrect transaction index in API responses.

• Add undefined check for transaction index.

---

Changes walkthrough 📝

	Relevant files
Bug fix	api.ts Correct transaction index and add undefined check                src/api.ts Correct transaction index calculation in API methods. Add check for undefined transaction index. +6/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 26a7124)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 90

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The check for result.transactionIndex assumes it is always present when result is an object. Ensure that this assumption is valid or handle cases where transactionIndex might be undefined. if (typeof result === 'object' && result.transactionIndex && args[1] !== undefined) { const transactionIndex = parseInt(args[1], 16); if (!isNaN(transactionIndex)) { result.transactionIndex = '0x' + transactionIndex.toString(16); }

[github-actions]

Persistent review updated to latest commit 26a7124

[shanejonas]

Can we investigate why it has transactionIndex: "0x1" from where its getting this from? this fix just overriding it without any explanation on the root cause. If its incorrect from the json-rpc-server ingestion or the collector API (or some other downstream service) we should find/fix it there.

[urnotsam]

@jintukumardas @shanejonas

its hard coded in like 8 places in shardeum server. Not sure what the downstream changes frmo that point on would be

[BelfordZ]

Did anyone actually check to see if its returning proper TX indexes now?

I can't see how this would fix the problem.

[jintukumardas]

Thanks for reviewing. This solution aligns with our existing approach for explorer responses where we override the value with user given index (see: [https://github.com/shardeum/json-rpc-server/blob/26a7124583f4406ce7a95fc63d1bca8b76acac95/src/api.ts#L221]). As Sam noted, this value is hardcoded everywhere and represents the transaction's index within the requested block. This allows for a quick fix since we are already fetching that index from the block response. I've locally verified these changes by pointing RPC to stagenet. The attached image shows a comparison: the left side displays block data from the explorer, and the right side shows the 3rd indexed transaction for the same block hash, confirming they match.

[BelfordZ]

ETH Handlers Testing

Summary of work done

• Created comprehensive test suites for two JSON-RPC handlers:
  • eth_getTransactionByBlockHashAndIndex
  • eth_getTransactionByBlockNumberAndIndex
• Fixed TypeScript errors in the handlers
• Identified and documented issues with callback usage and return values
• Achieved > 93% code coverage for both handlers

Test Coverage

• eth_getTransactionByBlockHashAndIndex: 93.33% statements, 80.76% branches, 100% functions, 97.05% lines
• eth_getTransactionByBlockNumberAndIndex: 93.58% statements, 82.14% branches, 100% functions, 97.10% lines

Issues Found

Two main issues were identified and documented as tickets:

1. Multiple callback calls - Both handlers call the callback function multiple times in certain code paths, which can lead to unexpected behavior for clients.

2. Inconsistent return values - When transactions aren't found, handlers sometimes return undefined instead of the expected null value specified in the Ethereum JSON-RPC specification.

Test Scenarios

The tests cover all major code paths and edge cases:

• Input validation failures
• Successful transaction retrieval via Collector API
• Transaction index adjustment
• Error handling for Collector API exceptions
• Explorer API fallback behavior
• Special block number values ('latest', 'earliest')
• Error handling for Explorer API
• Configuration state handling (disabled components)

Next Steps

• Address the identified issues in the tickets folder
• Implement similar test patterns for other ETH handlers
• Consider refactoring handlers to reduce code duplication and improve maintainability

[mssabr01]

Please remove the anys from the function prototypes in BuildGetTransactionByBlockHashAndIndex and BuildGetTransactionByBlockNumberAndIndex

[mssabr01]

Can we make this work without any?

[mssabr01]

Same question for all the as any below