Skip to content

refactor(batcher): retry function - #1273

Merged
JuArce merged 53 commits into
stagingfrom
retry-functions-refactor
Nov 4, 2024
Merged

refactor(batcher): retry function#1273
JuArce merged 53 commits into
stagingfrom
retry-functions-refactor

Conversation

@entropidelic

@entropidelicentropidelic commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

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.

@entropidelicentropidelic changed the title refactor: retry functionrefactor(batcher): retry functionOct 16, 2024
@avilagaston9avilagaston9 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
Comment threadbatcher/aligned-batcher/src/lib.rs Outdated
Comment threadbatcher/aligned-batcher/src/lib.rs Outdated
Comment threadbatcher/aligned-batcher/src/lib.rs
Comment threadbatcher/aligned-batcher/src/lib.rs Outdated
Comment threadbatcher/aligned-batcher/src/retry.rs Outdated
Comment threadbatcher/aligned-batcher/src/retry.rs Outdated
Comment threadbatcher/aligned-batcher/src/retry.rs
Comment threadbatcher/aligned-batcher/src/lib.rs
Comment threadbatcher/aligned-batcher/src/retry.rs Outdated
Comment threadbatcher/aligned-batcher/src/retry.rs

@JuArceJuArce left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment threadbatcher/aligned-batcher/src/lib.rs Outdated
Comment threadbatcher/aligned-batcher/src/retry/batcher_retryables.rs Outdated
Comment threadbatcher/aligned-batcher/src/retry/mod.rs
@avilagaston9
avilagaston9force-pushed the retry-functions-refactor branch from d556a33 to 7da5d3eCompareOctober 29, 2024 01:42
@github-actions

github-actionsBot commented Oct 29, 2024

Copy link
Copy Markdown

Changes to gas cost

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

🧾 Summary (10% most significant diffs)

ContractMethodAvg (+/-)%
AlignedLayerServiceManagercreateNewTask-123 ✅-0.17%

Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,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
avilagaston9force-pushed the retry-functions-refactor branch 2 times, most recently from 39fb306 to 7da5d3eCompareOctober 29, 2024 02:38
@JuArce
JuArce merged commit d5d8eb3 into stagingNov 4, 2024
@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 freeto 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

@entropidelic@Oppen@uri-99@JuArce@avilagaston9@MarcosNicolau@samoht9277