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(transactions): encode eip1559 #45

Merged
merged 8 commits into from
Jun 10, 2024
Merged

feat(transactions): encode eip1559 #45

merged 8 commits into from
Jun 10, 2024

Conversation

yash-atreya
Copy link
Member

Motivation

Addresses #44

Solution

PR Checklist

  • Added Documentation
  • Breaking changes

Copy link
Contributor

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good—anything blocking? let me know if you have bandwith, else I can take this over the line so we can focus on higher prio items


// Match hash
assert_eq!(*signed_tx.hash(), hash);
// TODO: Recover Signer - Enable feature k256 on consensus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anything blocking here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. As recover_signer is gated behind the k256, we're facing a similar issue to #26. Either need to make it default in alloy or fix the gating. cc @zerosnacks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha understood—are we tracking this issue somewhere? else i'll make it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think we're tracking. Could reopen #26, I believe we'll encounter this few more times as more examples are written.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reopened #26

use eyre::Result;
#[tokio::main]
async fn main() -> Result<()> {
// EIP1559 transaction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something cool would be to link to where the test vector was taken from (e.g etherscan link), that way ppl can also reproduce themselves

@zerosnacks
Copy link
Member

Any blockers? Would be great to get this in

@yash-atreya yash-atreya marked this pull request as ready for review June 4, 2024 15:34
@yash-atreya yash-atreya requested a review from zerosnacks as a code owner June 4, 2024 15:34
@yash-atreya
Copy link
Member Author

Any blockers? Would be great to get this in

@zerosnacks Sorry for the delay, can be merged

Comment on lines 13 to 43
// Signer of the transaction.
let signer = address!("DD6B8b3dC6B7AD97db52F08a275FF4483e024CEa");

let tx = TxEip1559 {
chain_id: 1,
nonce: 0x42,
gas_limit: 44386,
to: TxKind::Call( address!("6069a6c32cf691f5982febae4faf8a6f3ab2f0f6")),
value: U256::from(0_u64),
input: hex!("a22cb4650000000000000000000000005eee75727d804a2b13038928d36f8b188945a57a0000000000000000000000000000000000000000000000000000000000000000").into(),
max_fee_per_gas: 0x4a817c800,
max_priority_fee_per_gas: 0x3b9aca00,
access_list: AccessList::default(),
};

let sig = Signature::from_scalars_and_parity(
b256!("840cfc572845f5786e702984c2a582528cad4b49b2a10b9db1be7fca90058565"),
b256!("25e7109ceb98168d95b09b18bbf6b685130e0562f233877d492b94eee0c5b6d1"),
false,
)
.unwrap();

let signed_tx = tx.into_signed(sig);

// Match hash
assert_eq!(*signed_tx.hash(), hash);

let recovered_signer = signed_tx.recover_signer().unwrap();
assert_eq!(recovered_signer, signer);

Ok(())
Copy link
Member

@zerosnacks zerosnacks Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the goal of the example? To recover the signer from the transaction? Some additional comments would be great

For #44, is it within the scope of this PR to also show the other transaction types (are they different?)? If so, should we add specific files for each (like we do for send_*_transaction)

@zerosnacks
Copy link
Member

Any blockers? Would be great to get this in

@zerosnacks Sorry for the delay, can be merged

No worries!

Make sure to add it as an entry in the README.md as well

@zerosnacks zerosnacks merged commit cb77139 into main Jun 10, 2024
4 checks passed
@zerosnacks zerosnacks deleted the encode_decode_txs branch June 10, 2024 09:06
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.

4 participants