-
Notifications
You must be signed in to change notification settings - Fork 6
feat: support directly forward transactions to sequencer #265
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
CodSpeed Performance ReportMerging #265 will not alter performanceComparing Summary
|
frisitano
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.
Thanks, looks good! Left a question inline.
|
Please fix CI and then feel free to merge. |
Signed-off-by: Gregory Edison <[email protected]>
This reverts commit 27bd9da.
fb048ca to
499e9ae
Compare
|
@yiweichi can we merge this? |
will merge soon, just need to add a testing. |
3c5e0ab to
f3d9120
Compare
9baf144 to
9e11ec9
Compare
9e11ec9 to
9c750c0
Compare
frisitano
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.
Instead of modifying the constructor for EthTransactionValidatorBuilder we should use the pre-existing with_local_transactions_config to set the LocalTransactionConfig. This minimises the diff with upstream which is a priority for us.
frisitano
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.
Looks good. Left some comments inline around logging. We should prefix the target with scroll:: such that when we filter logs using RUST_LOG=scroll=trace we can correctly filter the appropriate target. Otherwise, these logs may get lost.
frisitano
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.
lgtm
Reth counterpart of l2geth scroll-tech/go-ethereum#1208.
Related rollup-node PR: scroll-tech/rollup-node#189
Implemented a
SequencerClient, which will help to forward transaction to sequencer's RPC directly(support both http and ws). Whether a transaction forwarding succeed or not, the transaction will be added to local transaction pool.