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

Use quantity for chainid key type #49

Closed
mattsse opened this issue Oct 16, 2024 · 7 comments · Fixed by #51
Closed

Use quantity for chainid key type #49

mattsse opened this issue Oct 16, 2024 · 7 comments · Fixed by #51
Labels
good first issue Good for newcomers

Comments

@mattsse
Copy link
Member

mattsse commented Oct 16, 2024

this should serialize chainid as quantity (U64)

pub struct WalletCapabilities(pub HashMap<ChainId, Capabilities>);

this could be made nicer if we had alloy-rs/alloy#1502

but for now we need to use the alloy-primitives::U64 type here instead of u64

TODO

  • use U64 as the key type
@mattsse mattsse added the good first issue Good for newcomers label Oct 16, 2024
@0xurb
Copy link
Contributor

0xurb commented Oct 16, 2024

request (alloy_rpc_types::TransactionRequest) chainId is u64 tho

@mattsse
Copy link
Member Author

mattsse commented Oct 16, 2024

ah sorry my issue was vague, I was referring to serde here

HashMap<ChainId, Capabilities>

should serialize chainid keys as quantity (hex)

@0xurb
Copy link
Contributor

0xurb commented Oct 17, 2024

Should be useful to add “blocked” issue label to the labels list and point here maybe

@onbjerg
Copy link
Member

onbjerg commented Oct 17, 2024

it's not blocked, we can simply use U64 instead of ChainId

@0xurb
Copy link
Contributor

0xurb commented Oct 17, 2024

It’s awaiting quantity hashmap from alloy to put serde attribute on WalletCapabilities.

@onbjerg
Copy link
Member

onbjerg commented Oct 17, 2024

No, U64 is always encoded as a quantity in alloy:) so just using U64 solves this issue

@0xurb
Copy link
Contributor

0xurb commented Oct 17, 2024

Oh, yes, my fault :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants