-
Notifications
You must be signed in to change notification settings - Fork 391
hotfix: make aggregator fee multiplier customizable #1690
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
Conversation
MarcosNicolau
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me.
PatStiles
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worked locally LGTM
0a4860f to
263f5bd
Compare
avilagaston9
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are debugging an issue when trying to submit only one proof
avilagaston9
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We understood the problem. It wasn't an issue!
|
I think we should also update |
batcher.aggregator_fee_percentage_multiplier, usingDEFAULT_AGGREGATOR_FEE_PERCENTAGE_MULTIPLIERas a default if not set;AGGREGATOR_GAS_COSTandCONSTANT_GAS_COSTtoDEFAULT_AGGREGATOR_GAS_COSTandDEFAULT_CONSTANT_GAS_COSTrespectively;batcher.aggregator_fee_percentage_multiplierconfig andbatcher.aggregator_gas_cost;Testing
Basic testing involves just the regular flow, start a devnet, send some proofs, check that they verify and batches make it to L1. The expected behavior is cheaper costs, in the telemetry it should show changes vs testnet in
fee per proof.To test more in depth, try:
batcher.aggregator_fee_percentage_multiplier, it should work and charge the same (the defaults in the example config files and the constant match);Each of these changes should require only a batcher restart, not recompilation and not restarting of other components.
Same for
batcher.aggregator_gas_cost, which is 330k by default.Type of change
Checklist
testnet, everything else tostaging