-
Notifications
You must be signed in to change notification settings - Fork 390
feat(provider): Add eth_sendBundle support to provider #2556
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
base: main
Are you sure you want to change the base?
Conversation
Should this be in a separate repo and under an extension trait? alloy-rs/mev? |
I think this question goes in the direction of the maintainers, but I would suggest keeping it here in |
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.
this makes sense, imo having a way to easily install additional headers is very useful so we should add this now that we have the extension
smol nits and a question
/// Returns a [`HeaderMap`] from the request if `HttpHeaderExtension` is present; | ||
/// otherwise, returns an empty map. Only supported for single requests. | ||
pub fn headers(&self) -> HeaderMap { | ||
if let Some(single_req) = self.as_single() { | ||
if let Some(http_header_extension) = | ||
single_req.meta().extensions().get::<HttpHeaderExtension>() | ||
{ |
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.
I think this makes sense as a workaround because this is a common use case,
can we add helper fn for retrieving the HttpHeaderExtension
maybe even as mut so that the user doesnt have to deal with the extension type directly
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.
Yes this would be a great addition in the future
crates/rpc-client/src/call.rs
Outdated
/// Maps the metadata of the request using the provided function. | ||
pub fn map_meta(self, f: impl FnOnce(RequestMeta) -> RequestMeta) -> Self { |
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.
this is public but can panic, this feels wrong, why do we need this?
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.
This is a valid point, better be safe then sorry. I was initially inspired by the map_params
fn above and just copied it. I guess it was intended as unreachable!
, but I introduces now an error type here. I think it is reasonable.
This here should be refactored as well, but I like to keep the PR small. But I am ok to pick this up and make the use of RequestAlreadySentError
here as well:
pub fn map_params<NewParams: RpcSend>(
self,
map: impl Fn(Params) -> NewParams,
) -> RpcCall<NewParams, Resp, Output, Map> {
let CallState::Prepared { request, connection } = self.state else {
panic!("Cannot get request after request has been sent");
};
...
body: String, | ||
signer: &S, | ||
) -> Result<String, alloy_signer::Error> { | ||
let message_hash = keccak256(body.as_bytes()).to_string(); |
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.
The signature generation appears to be using an incorrect approach for Flashbots authentication. According to Flashbots documentation, the raw message bytes should be signed directly, not a string representation of the hash. The current implementation:
let message_hash = keccak256(body.as_bytes()).to_string();
let signature = signer.sign_message(message_hash.as_bytes()).await?;
should be simplified to:
let signature = signer.sign_message(body.as_bytes()).await?;
This would correctly sign the raw JSON request body as expected by Flashbots relay servers. The current approach is performing an extra hashing step and string conversion that would produce an incompatible signature.
let message_hash = keccak256(body.as_bytes()).to_string(); | |
let signature = signer.sign_message(body.as_bytes()).await?; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
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.
This is not correct. It would fail with "error in signature check...". Guess in has worked with ethers that way
@mattsse PR is ready to be reviewed again. I plan to add other methods now as well and challenge the data model a bit to make it more robust/compatible with different builder implementations. But would do this in separate PRs. |
This adds a new method + builder to the provider:
send_bundle
: send a MEV bundleThis feature also introduces custom http headers for a http request. For that the extension feature is used and a custom type
HttpHeaderExtension
is introduces. A transport can choose to support it e.g.transport-http
.For flexibility
FLASHBOTS_SIGNATURE_HEADER
andsign_flashbots_payload
are exported, if someone prefers to build the request manually.Usage:
Find here a full example how it works without this feature: https://github.com/cakevm/alloy-flashbots-client/blob/main/src/main.rs