Skip to content
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.

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

feat: enabled fast return on eth_sendRawTransaction #3273

Conversation

quiet-node
Copy link
Member

@quiet-node quiet-node commented Nov 18, 2024

Description:

The eth_sendRawTransaction endpoint currently faces timeouts from various sources, including server, network, and client settings. For large callData, it uses Hedera File Service (HFS) to create and append file transactions, increasing latency and degrading user experience.

This PR introduces a new conceptual feature design for eth_sendRawTransaction, enabling the API to return the transaction hash immediately upon passing prechecks. All subsequent processing (e.g., broadcasting to the network, handling file transactions, polling the mirror node, tracking HBAR consumption, etc.) is handled asynchronously in the background.

The primary impact of this feature is a significant reduction in end-to-end client wait times while ensuring backend integrity remains intact.

Important Note for Clients:

With the transaction hash returned instantly, clients must monitor the transaction status using this hash (e.g., via eth_getTransactionReceipt or monitoring confirmations).

If a request encounters internal errors (e.g., exceeding maxChunkSize, HBAR limits, SDK errors), these will no longer appear in the response. Instead, a transaction hash is returned, and eth_getTransactionReceipt may return null indefinitely. That said, it remains the responsibility of the users to handle such cases appropriately.

Fixes #3270

Related issue(s):

Additional aspects and considerations of the feature are highlighted in the ongoing discussion in #3202. Please feel free to share your insights in the thread if you have any.

Notes for reviewer:

TL;DR; The primary change in this PR is a refactor, mainly within eth.ts. The remaining work involves updating tests to align with the new immediate return logic. Most efforts are directed toward adding polling logic to verify the validity of transactions before proceeding to the next instruction.

  • The feature is designed for backward compatibility, allowing it to be toggled on or off while ensuring the end-to-end flow remains consistent.
  • A new global configuration, USE_ASYNC_TX_PROCESSING, acts as the feature flag for this functionality.
  • The EthImpl.sendRawTransaction method has been refactored to extract transaction processing logic into a new function, sendRawTransactionTxProcessor(). When USE_ASYNC_TX_PROCESSING is enabled, sendRawTransactionTxProcessor() is called without await, allowing transaction processing to run asynchronously in the background. If the flag is disabled, the function is awaited, maintaining the original synchronous flow.
  • The remainder of the work focuses on updating tests to accommodate the new feature.

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Copy link

github-actions bot commented Nov 18, 2024

Test Results

 22 files  +  3  309 suites  +64   1h 1m 14s ⏱️ + 31m 40s
606 tests  -   5  597 ✅  -   6  4 💤 ±0  5 ❌ +1 
819 runs  +187  809 ✅ +185  4 💤 ±0  6 ❌ +2 

For more details on these failures, see this check.

Results for commit d274e6b. ± Comparison against base commit 7accf1f.

This pull request removes 9 and adds 4 tests. Note that renamed tests count towards both.
"before all" hook for "should individually update amountSpents of different spending plans" ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch2 Multiple users with different tiers "before all" hook for "should individually update amountSpents of different spending plans"
"before each" hook for "Should eventually exhaust the hbar limit for EXTENDED user and still allow another EXTENDED user to make calls" ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch2 Preconfigured Tiers EXTENDED Tier "before each" hook for "Should eventually exhaust the hbar limit for EXTENDED user and still allow another EXTENDED user to make calls"
"before each" hook for "Should increase the amount spent of the spending plan by the transaction cost" ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch2 Preconfigured Tiers PRIVILEGED Tier "before each" hook for "Should increase the amount spent of the spending plan by the transaction cost"
@release fail "eth_getTransactionReceipt" on precheck with wrong nonce error when sending a tx with a higher nonce ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-1 RPC Server Acceptance Tests RPC Server Acceptance Tests Transaction related RPC Calls Prechecks @release fail "eth_getTransactionReceipt" on precheck with wrong nonce error when sending a tx with a higher nonce
Should eventually exhaust the total HBAR limits after many large contract deployments by different tiered users ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch2 Multiple users with different tiers Should eventually exhaust the total HBAR limits after many large contract deployments by different tiered users
should create a BASIC spending plan for a new user and use the same plan on second transaction and different plan on third transaction from another user ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch1 BASIC Tier should create a BASIC spending plan for a new user and use the same plan on second transaction and different plan on third transaction from another user
should eventually exhaust the hbar limit for a BASIC user after multiple deployments of large contracts ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch1 BASIC Tier should eventually exhaust the hbar limit for a BASIC user after multiple deployments of large contracts
should execute "eth_sendRawTransaction" and deploy a contract with more than max transaction fee ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-1 RPC Server Acceptance Tests RPC Server Acceptance Tests Transaction related RPC Calls should execute "eth_sendRawTransaction" and deploy a contract with more than max transaction fee
should individually update amountSpents of different spending plans ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch2 Multiple users with different tiers should individually update amountSpents of different spending plans
Should eventually exhaust the total HBAR limits after many large contract deployments ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch1 Multiple users with different tiers Should eventually exhaust the total HBAR limits after many large contract deployments
should create a BASIC spending plan for a new user and use the same plan on second transaction and different plan on third transaction from another user ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch2 BASIC Tier should create a BASIC spending plan for a new user and use the same plan on second transaction and different plan on third transaction from another user
should eventually exhaust the hbar limit for a BASIC user after multiple deployments of large contracts ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch2 BASIC Tier should eventually exhaust the hbar limit for a BASIC user after multiple deployments of large contracts
should individually update amountSpents of different spending plans ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests HBAR Rate Limit For Different Spending Plan Tiers @hbarlimiter-batch1 Multiple users with different tiers should individually update amountSpents of different spending plans

♻️ This comment has been updated with latest results.

@quiet-node quiet-node force-pushed the 3270-eth_sendrawtransaction-fast-return-add-feature-flag-and-move-all-transaction-processing-logic-in-background-if-the-feature-flag-is-enabled branch 2 times, most recently from 4dfec8f to 239b1fa Compare November 19, 2024 17:44
@quiet-node quiet-node changed the title draft: enabled fast return on eth_sendRawTransaction feat: enabled fast return on eth_sendRawTransaction Nov 19, 2024
@quiet-node quiet-node force-pushed the 3270-eth_sendrawtransaction-fast-return-add-feature-flag-and-move-all-transaction-processing-logic-in-background-if-the-feature-flag-is-enabled branch 3 times, most recently from 11c9a39 to 3fa0638 Compare November 19, 2024 19:50
Copy link
Member Author

@quiet-node quiet-node left a comment

Choose a reason for hiding this comment

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

Left some notes and questions for reviewers.

package.json Show resolved Hide resolved
packages/relay/package.json Show resolved Hide resolved
packages/relay/src/lib/eth.ts Show resolved Hide resolved
packages/server/tests/acceptance/rpc_batch1.spec.ts Outdated Show resolved Hide resolved
@quiet-node quiet-node marked this pull request as ready for review November 19, 2024 20:23
victor-yanev
victor-yanev previously approved these changes Nov 20, 2024
@quiet-node quiet-node force-pushed the 3270-eth_sendrawtransaction-fast-return-add-feature-flag-and-move-all-transaction-processing-logic-in-background-if-the-feature-flag-is-enabled branch from 323ded7 to 8950af9 Compare November 23, 2024 18:34
@natanasow
Copy link
Collaborator

LG, several tests are failing due to an undefined variable. Let's fix them before the final approvals.

Signed-off-by: Logan Nguyen <[email protected]>
Copy link

sonarcloud bot commented Nov 25, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
5.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@quiet-node
Copy link
Member Author

LG, several tests are failing due to an undefined variable. Let's fix them before the final approvals.

@natanasow Thanks for the good catch! Missed that during rebasing! Updated!

Copy link
Collaborator

@natanasow natanasow left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the great work!

@quiet-node quiet-node merged commit 56903ba into main Nov 25, 2024
43 of 46 checks passed
@quiet-node quiet-node deleted the 3270-eth_sendrawtransaction-fast-return-add-feature-flag-and-move-all-transaction-processing-logic-in-background-if-the-feature-flag-is-enabled branch November 25, 2024 21:35
Copy link

codecov bot commented Dec 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.92%. Comparing base (7accf1f) to head (d274e6b).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3273      +/-   ##
==========================================
+ Coverage   77.82%   77.92%   +0.09%     
==========================================
  Files          66       66              
  Lines        4474     4476       +2     
  Branches     1005     1004       -1     
==========================================
+ Hits         3482     3488       +6     
+ Misses        617      615       -2     
+ Partials      375      373       -2     
Flag Coverage Δ
config-service 98.14% <ø> (ø)
relay 78.67% <100.00%> (+0.12%) ⬆️
server 83.28% <ø> (ø)
ws-server 36.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ckages/config-service/src/services/globalConfig.ts 100.00% <ø> (ø)
packages/relay/src/lib/eth.ts 71.28% <100.00%> (+0.21%) ⬆️
packages/relay/src/utils.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
5 participants