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

fix: reduced TTL for feeHistory, getTinyBarGasFee, and getPrice #3482

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

simzzz
Copy link
Contributor

@simzzz simzzz commented Feb 17, 2025

Description:
This PR reduces the TTL for caching fee history, gas fee, and gas price in an attempt to reduce the likelihood of INSUFFICIENT_TX_FEE errors.

We suspect that a reason that some transactions fail is because there is a fluctuation on the network gas price, while we return the cached value resulting in a too low gas estimation. By reducing the TTL in a few places where the gas fee is used it would reduce the likelihood of such problem.

Related issue(s):
#3283

Notes for reviewer:

Checklist

@simzzz simzzz requested review from a team as code owners February 17, 2025 16:49
@simzzz simzzz requested a review from lukelee-sl February 17, 2025 16:49
Copy link

github-actions bot commented Feb 17, 2025

Test Results

 20 files   -   8  290 suites   - 113   35m 27s ⏱️ - 1h 1m 20s
619 tests + 33  611 ✅ + 50  4 💤 +2  4 ❌  - 19 
701 runs   - 414  693 ✅  - 386  4 💤  - 2  4 ❌  - 26 

For more details on these failures, see this check.

Results for commit f5035eb. ± Comparison against base commit 4406824.

This pull request removes 8 and adds 41 tests. Note that renamed tests count towards both.
"before all" hook for "emits an approval event" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests HTS token should behave like erc20 transfer from when the token owner is not the zero address when the recipient is not the zero address when the spender has enough allowance "before all" hook for "emits an approval event"
"before all" hook for "reverts" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests HTS token should behave like erc20 transfer from when the token owner is not the zero address when the recipient is not the zero address when the spender does not have enough allowance when the token owner has enough balance "before all" hook for "reverts"
"before all" hook for "should execute "eth_getCode" for hts token" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode "before all" hook for "should execute "eth_getCode" for hts token"
"before all" hook in "Debug API Test Suite" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite "before all" hook in "Debug API Test Suite"
"before all" hook in "HBAR Rate Limit Tests" ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests "before all" hook in "HBAR Rate Limit Tests"
"before each" hook for "should execute "eth_getStorageAt" request to get current state changes without passing block" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before each" hook for "should execute "eth_getStorageAt" request to get current state changes without passing block"
"before each" hook for "should execute "eth_getUncleByBlockHashAndIndex"" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before each" hook for "should execute "eth_getUncleByBlockHashAndIndex""
"before each" hook for "should return 0x0 for account evm_address on eth_getCode" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before each" hook for "should return 0x0 for account evm_address on eth_getCode"
"before all" hook in "@web-socket-batch-3 eth_subscribe newHeads" ‑ RPC Server Acceptance Tests Acceptance tests @web-socket-batch-3 eth_subscribe newHeads "before all" hook in "@web-socket-batch-3 eth_subscribe newHeads"
"before each" hook: reducing balance for "reverts" ‑ RPC Server Acceptance Tests Acceptance tests @erc20 Acceptance Tests HTS token should behave like erc20 transfer from when the token owner is not the zero address when the recipient is not the zero address when the spender does not have enough allowance when the token owner does not have enough balance "before each" hook: reducing balance for "reverts"
@release should be able to debug a failing CALL transaction of type 1559 with call depth and onlyTopCall false ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite Positive scenarios Test transactions of type: 2 @release should be able to debug a failing CALL transaction of type 1559 with call depth and onlyTopCall false
@release should be able to debug a failing CREATE transaction of type 1559 with call depth and onlyTopCall false ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite Positive scenarios Test transactions of type: 2 @release should be able to debug a failing CREATE transaction of type 1559 with call depth and onlyTopCall false
@release should be able to debug a successful CALL transaction of type 1559 with call depth and onlyTopCall true ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite Positive scenarios Test transactions of type: 2 @release should be able to debug a successful CALL transaction of type 1559 with call depth and onlyTopCall true
@release should be able to debug a successful CREATE transaction of type 1559 with call depth and onlyTopCall true ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite Positive scenarios Test transactions of type: 2 @release should be able to debug a successful CREATE transaction of type 1559 with call depth and onlyTopCall true
from/to Addresses in transaction between accounts are in evm format ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests Formats of addresses in Transaction and Receipt results from/to Addresses in transaction between accounts are in evm format
from/to Addresses in transaction to a contract (deployed through HAPI tx) are in evm and long-zero format ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests Formats of addresses in Transaction and Receipt results from/to Addresses in transaction to a contract (deployed through HAPI tx) are in evm and long-zero format
from/to Addresses in transaction to a contract (deployed through the relay) are in evm and long-zero format ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests Formats of addresses in Transaction and Receipt results from/to Addresses in transaction to a contract (deployed through the relay) are in evm and long-zero format
from/to Addresses when transferring HTS tokens to the tokenAddress are in evm and long-zero format ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests Formats of addresses in Transaction and Receipt results from/to Addresses when transferring HTS tokens to the tokenAddress are in evm and long-zero format
…

♻️ This comment has been updated with latest results.

@simzzz simzzz added the bug Something isn't working label Feb 18, 2025
@simzzz simzzz added this to the 0.67.0 milestone Feb 18, 2025
@simzzz simzzz self-assigned this Feb 18, 2025
@quiet-node
Copy link
Member

@simzzz, the PR description doesn’t explain how lowering the TTL would help resolve the issue. Additionally, there are no end-to-end tests to verify the fix. Could you update the description with more details on how this change addresses the problem? If possible, adding e2e tests would be beneficial to prevent regressions. Thanks!

Signed-off-by: Simeon Nakov <[email protected]>
Signed-off-by: Simeon Nakov <[email protected]>
@simzzz simzzz changed the title fix: reduced TTL for feeHistory and getTinyBarGasFee fix: reduced TTL for feeHistory, getTinyBarGasFee, and getPrice Feb 20, 2025
Copy link

codecov bot commented Feb 20, 2025

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.61%. Comparing base (4406824) to head (f5035eb).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
packages/relay/src/lib/clients/sdkClient.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3482      +/-   ##
==========================================
+ Coverage   85.17%   85.61%   +0.44%     
==========================================
  Files          69       69              
  Lines        4734     4735       +1     
  Branches      999      999              
==========================================
+ Hits         4032     4054      +22     
+ Misses        409      396      -13     
+ Partials      293      285       -8     
Flag Coverage Δ
config-service 95.16% <ø> (ø)
relay 79.55% <87.50%> (+<0.01%) ⬆️
server 83.60% <ø> (ø)
ws-server 36.31% <ø> (ø)

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

Files with missing lines Coverage Δ
packages/relay/src/lib/constants.ts 100.00% <100.00%> (ø)
packages/relay/src/lib/eth.ts 86.37% <100.00%> (+0.75%) ⬆️
packages/relay/src/lib/clients/sdkClient.ts 69.39% <50.00%> (ø)

... and 66 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants