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

[Feature] Add tree hash root for BidTrace #1024

Open
ferranbt opened this issue Jul 8, 2024 · 5 comments
Open

[Feature] Add tree hash root for BidTrace #1024

ferranbt opened this issue Jul 8, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@ferranbt
Copy link

ferranbt commented Jul 8, 2024

Component

rpc

Describe the feature you would like

Generate the tree hash root for the BidTrace. In order to send the Bid to the relayer, the builder has to sign the hash tree root of the BidTrace.

Additional context

No response

@ferranbt ferranbt added the enhancement New feature or request label Jul 8, 2024
@ferranbt
Copy link
Author

ferranbt commented Jul 8, 2024

I think it could be as simple as:

#[cfg_attr(feature = "ssz_hash", derive(tree_hash_derive::TreeHash))]
pub struct BidTrace {

and

# ssz_hash
tree_hash = { version = "0.6", optional = true }
tree_hash_derive = { version = "0.6", optional = true }

[features]
...
ssz_hash = [
    "dep:tree_hash",
    "dep:tree_hash_derive"
]

But, we need to add the TreeHash trait impl for Address and Uint<> type in core too? Should I open a PR with the changes there first?

@mattsse
Copy link
Member

mattsse commented Jul 8, 2024

makes sense, although, this crate currently only supports the ethereum_primitives: https://github.com/sigp/tree_hash https://github.com/sigp/tree_hash/blob/93d0e3ff58fb174f84dea8d2b5374787b7a4c85a/tree_hash/src/impls.rs#L2

the impls don't look to complex and I think we could easily feature gate them in alloy-primitives, like ssz

but it's not ideal because the treehash API uses ethereum-types h256 ...

fyi @DaniPopes

@DaniPopes
Copy link
Member

DaniPopes commented Jul 8, 2024

If this gets released we don't need to add derives in alloy core sigp/tree_hash#16

@mattsse
Copy link
Member

mattsse commented Jul 8, 2024

oh nice!

@michaelsproul
Copy link

we (upstream tree_hash) will hopefully make progress on the alloy migration in the coming weeks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants