Skip to content

Conversation

@jakim929
Copy link
Contributor

@jakim929 jakim929 commented Jul 30, 2024

Motivation

tx hash calculation for deposit tx for anvil --optimism was different from specs. this made it difficult to deterministically generate the L2 tx hash from the L1 deposit transaction.

  • the EIP-2718 tx type was not included when calculating hash
  • encoding was slightly different from spec
    • nonce was included in the rlp encoding
    • order of fields was different from spec

Solution

  • calculate tx hash after encoding in eip-2718 envelope format
  • fix order of fields / remove nonce when RLP encoding / decoding
  • added tests - checks against a op-sepolia tx hash / fields to make sure that the hash / encoding is consistent

Test to repro issue

the following test would fail previously

    fn test_decode_encode_deposit_tx() {
        // https://sepolia-optimism.etherscan.io/tx/0xbf8b5f08c43e4b860715cd64fc0849bbce0d0ea20a76b269e7bc8886d112fca7
        let tx_hash: TxHash = "0xbf8b5f08c43e4b860715cd64fc0849bbce0d0ea20a76b269e7bc8886d112fca7"
            .parse::<TxHash>()
            .unwrap();

        // https://sepolia-optimism.etherscan.io/getRawTx?tx=0xbf8b5f08c43e4b860715cd64fc0849bbce0d0ea20a76b269e7bc8886d112fca7
        let raw_tx = alloy_primitives::hex::decode(
           "7ef861a0dfd7ae78bf3c414cfaa77f13c0205c82eb9365e217b2daa3448c3156b69b27ac94778f2146f48179643473b82931c4cd7b8f153efd94778f2146f48179643473b82931c4cd7b8f153efd872386f26fc10000872386f26fc10000830186a08080",
        )
        .unwrap();
        let dep_tx = TypedTransaction::decode(&mut raw_tx.as_slice()).unwrap();
        let mut encoded = Vec::<u8>::new();
        dep_tx.encode_2718(&mut encoded);
        assert_eq!(raw_tx, encoded);
        assert_eq!(tx_hash, dep_tx.hash());
    }

Copy link
Member

@mattsse mattsse 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 great, ty

few pedantic nits

we now have this type also here https://github.com/alloy-rs/op-alloy so we could replace the entire anvil deposit type, will track this separately.

q, are you using eth_sendraw deposit txs at OP for testing?

@jakim929
Copy link
Contributor Author

thanks for the quick review! 🙏 addressed the requested changes

q, are you using eth_sendraw deposit txs at OP for testing?

Yep, it's very useful for testing bridging! and we're using it pretty extensively for https://github.com/ethereum-optimism/supersim , which is a local environment for testing L1 + multiple L2s (anvils) for bridging, message passing etc.

Also we're happy to help maintain the optimism related features on anvil including tasks like

we now have this type also here https://github.com/alloy-rs/op-alloy so we could replace the entire anvil deposit type, will track this separately.

and improving support for deposit tx validation in the tx pool.

@jakim929 jakim929 requested a review from mattsse July 30, 2024 21:29
@mattsse mattsse merged commit c998542 into foundry-rs:master Jul 31, 2024
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.

2 participants