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(da-throttling): Forward DA throttling and other miner_ requests to the builder #61

Closed
wants to merge 14 commits into from

Conversation

0xKitsune
Copy link

@0xKitsune 0xKitsune commented Jan 27, 2025

ref #48

This PR introduces logic to support DA throttling by forwarding miner_ requests to the builder.

For miner_setMaxDALimit, the MinerApiExtServer from op-alloy-rpc-jsonrpsee was implemented. Since the MinerApi
from Reth is synchronous
and the rollup-boost implementation has async calls, I recreated the trait within rollup-boost to be async. We can open an issue on Reth and use the trait from reth-rpc-api once this is upstreamed.

Feel free to let me know any feedback on the approach and if you have a preference on how to extend or amend the current test harness to best test this feature.

@ferranbt
Copy link
Collaborator

Hey! PR looks good, I will wait for @avalonche to give the final approval. What do you think about #62 to simplify the boilerplate require to add pure forwarding methods?

@0xOsiris
Copy link

Hey! PR looks good, I will wait for @avalonche to give the final approval. What do you think about #62 to simplify the boilerplate require to add pure forwarding methods?

For visibility we are running into some issues with the batcher shutting down when integration testing this PR. Let's hold off on merging until we have this resolved.

2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=warn msg="main loop returning"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=info msg="Receipt processing loop done"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=error msg="SetMaxDASize rpc failed, retrying." err="Post \"http://172.16.0.24:8541/\": context canceled"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=info msg="DA throttling loop done"
2025-01-28 22:07:20 t=2025-01-29T05:07:20+0000 lvl=info msg="Batch Submitter stopped"

* merge all api modules

* fix import

* simplify error handling

* update RollupBoostClient to RollupBoostServer

* remove unused error

* fix imports

* update function name for clarity

* add miner api ext server

* update into_merged_rpc, wip test

* wip tests

* update to use try_into

* expose rollup boost as a lib

* add temp logging

* simplifying changes

* remove test, update logging
@trianglesphere
Copy link

For visibility we are running into some issues with the batcher shutting down when integration testing this PR. Let's hold off on merging until we have this resolved.

That set of logs looks like a race condition between the different loops. The shutdown context is cancelled while the SetMaxDASize is happening. Once it finishes those calls it enters another loop where if checks if the shutdown context is cancelled or not.

@0xKitsune
Copy link
Author

Closing in favor of #69

@0xKitsune 0xKitsune closed this Jan 31, 2025
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.

6 participants