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

Implement RLP decoding for transactions #805

Merged
merged 2 commits into from Jan 30, 2022
Merged

Conversation

Rjected
Copy link
Contributor

@Rjected Rjected commented Jan 18, 2022

Motivation

Currently there is no way to decode a transaction from bytes, only encode to bytes.

Solution

Implemented the Decodable trait for:

  • TypedTransaction
    • TransactionRequest
    • Eip2930TransactionRequest
    • Eip1559TransactionRequest
  • NameOrAddress
  • Transaction

This change uses rlp_derive for the Decodable implementation on AccessListItem and AccessList.

This also adjusts each variant of TypedTransaction to contain the chain_id, and removes chain_id it from the rlp_signed and sighash functions.

Fixes #561

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@gakonst
Copy link
Owner

gakonst commented Jan 18, 2022

Seems like something is missing when testing against a live network?

failures:
281

282
---- nonce_manager stdout ----
283
thread 'nonce_manager' panicked at 'called Result::unwrap() on an Err value: MiddlewareError(MiddlewareError(JsonRpcClientError(JsonRpcError(JsonRpcError { code: -32602, message: "transaction could not be decoded: chainId is required for EIP-2718 style signatures", data: None }))))', ethers-

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

directionally this looks great - just think there's some decoding bug in testnet so i'd look at that

@gakonst
Copy link
Owner

gakonst commented Jan 28, 2022

@Rjected hey just checking up here, what do your plans look like for getting this over the line? can we help?

@rakita
Copy link
Contributor

rakita commented Jan 28, 2022

Didnt dive into code, but maybe this can help (or at least act as reference). It is impl of types transaction with good licence:
https://github.com/gnosis/reth/tree/rakita/txpool/tx_score/crates/core/src/transaction

@Rjected
Copy link
Contributor Author

Rjected commented Jan 28, 2022

Hey, yeah I have been pretty busy but I fixed the failing tests. Since the workflow needs to be approved, here's the workflow on my fork: https://github.com/Rjected/ethers-rs/actions/runs/1763919095.

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

sick

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.

Deserialising a raw signed transaction?
3 participants