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: bump gas price 10% for batcher #1261

Open
wants to merge 2 commits into
base: staging
Choose a base branch
from

Conversation

uri-99
Copy link
Contributor

@uri-99 uri-99 commented Oct 15, 2024

This PR

  • Bumps 10% the gas price calculated by the batcher, to avoid transactions hanged in the mempool.
  • Refactors names of constants, as multiplier/divider is unnecesarily complex, while a simple multiplier/100 is enough
  • Refactored batcher, now uses constants defined in SDK properly, instead of multiple definitions of this variable.

To Test

This changes the SDK and the Batcher.
So, test normal flow

@uri-99 uri-99 linked an issue Oct 15, 2024 that may be closed by this pull request
@uri-99 uri-99 changed the base branch from testnet to staging October 15, 2024 20:22
Copy link
Collaborator

@avilagaston9 avilagaston9 left a comment

Choose a reason for hiding this comment

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

LGTM!

/ U256::from(RESPOND_TO_TASK_FEE_LIMIT_DIVIDER);
* U256::from(RESPOND_TO_TASK_FEE_LIMIT_PERCENTAGE_MULTIPLIER))
/ U256::from(PERCENTAGE_DIVIDER);
let final_gas_price = gas_price * U256::from(GAS_PRICE_PERCENTAGE_MULTIPLIER)
Copy link
Collaborator

Choose a reason for hiding this comment

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

final_gas_price does not tell anything about the modification applied to the gas price

Also we should discuss where should be done this kind of actions because, the incremented gas price is being propagated across the function calls, but fee_params.gas_price does not say anything about a previous increment of 10%

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, gas price increment of 10% should be done here

//eth/mod.rs

pub async fn try_create_new_task(
...
) ... {  
...
.gas_price(fee_params.gas_price); <- here
...
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

One more thing, with the new system of retries, this can change too

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.

Bump base Fee 10% for Batcher
3 participants