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

refactor(batcher): retry function #1273

Merged
merged 53 commits into from
Nov 4, 2024
Merged

refactor(batcher): retry function #1273

merged 53 commits into from
Nov 4, 2024

Conversation

entropidelic
Copy link
Contributor

@entropidelic entropidelic commented Oct 16, 2024

Motivation

This PR implements the retry_function(), which receives a function to be retried using exponential backoff.

Description

Modifies the batcher to use retry_function() in the following calls:

  • user_balance_is_unlocked
  • get_user_nonce_from_ethereum
  • get_user_balance
  • get_gas_price
  • s3::upload_object
  • listen_new_blocks()

Stops using our forked version of ethers-rs since is not longer needed.

Observations

The calls within the Batcher::new() function are not modified since in case of failure the program will panic (changes can be implemented if needed).
Also create_new_task was left for a different PR.

All modified calls use the following default parameters:

  • DEFAULT_MIN_DELAY = 500
  • DEFAULT_MAX_TIMES = 5
  • DEFAULT_FACTOR = 2.0

How to Test

make anvil_start_with_block_time
make batcher_start_local
make batcher_send_burst_groth16

Now try killing the container running the s3 service. You should see 5 warnings separated by exponential time:

[2024-10-17T21:14:09Z WARN  aligned_batcher] Error uploading batch to s3 dispatch failure
  • There are also some test in batcher/aligned-batcher/src/retry.rs

  • You can also add a sleep() before the function you want to test in the batcher, and then kill Anvil before the call happens.

@entropidelic entropidelic changed the title refactor: retry function refactor(batcher): retry function Oct 16, 2024
@avilagaston9 avilagaston9 self-assigned this Oct 17, 2024
@avilagaston9 avilagaston9 marked this pull request as ready for review October 17, 2024 18:49
@avilagaston9 avilagaston9 marked this pull request as draft October 17, 2024 21:50
batcher/aligned-batcher/src/lib.rs Show resolved Hide resolved
batcher/aligned-batcher/src/lib.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/retry.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/retry.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/retry.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/lib.rs Show resolved Hide resolved
batcher/aligned-batcher/src/retry.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/retry.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@JuArce JuArce left a comment

Choose a reason for hiding this comment

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

batcher/aligned-batcher/src/lib.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/retry/batcher_retryables.rs Outdated Show resolved Hide resolved
batcher/aligned-batcher/src/retry/mod.rs Show resolved Hide resolved
Copy link

github-actions bot commented Oct 29, 2024

Changes to gas cost

Generated at commit: 2d736daf8205b0547c489cde3b1d6e0b3fb8b041, compared to commit: 192a535aa39bbad8cada32b9460bf59e33081685

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
AlignedLayerServiceManager createNewTask -123 ✅ -0.17%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
AlignedLayerServiceManager 4,771,213 (0) createNewTask
receive
53,923 (-12)
21,169 (0)
-0.02%
0.00%
73,843 (-123)
44,690 (-187)
-0.17%
-0.42%
74,117 (+66)
45,064 (0)
+0.09%
0.00%
74,786 (-84)
45,064 (0)
-0.11%
0.00%
256 (0)
256 (0)

@avilagaston9 avilagaston9 force-pushed the retry-functions-refactor branch 2 times, most recently from 39fb306 to 7da5d3e Compare October 29, 2024 02:38
@JuArce JuArce merged commit d5d8eb3 into staging Nov 4, 2024
6 checks passed
@JuArce JuArce deleted the retry-functions-refactor branch November 4, 2024 14:37
PatStiles pushed a commit that referenced this pull request Nov 6, 2024
Co-authored-by: avilagaston9 <gavila@fi.uba.ar>
Co-authored-by: samoht9277 <totobighouse@gmail.com>
Co-authored-by: Urix <43704209+uri-99@users.noreply.github.com>
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.

7 participants