diff --git a/test/integration/evmone-cli/t8n/CMakeLists.txt b/test/integration/evmone-cli/t8n/CMakeLists.txt index 4ab1e75cdc..30e99880a5 100644 --- a/test/integration/evmone-cli/t8n/CMakeLists.txt +++ b/test/integration/evmone-cli/t8n/CMakeLists.txt @@ -248,7 +248,7 @@ set_tests_properties( add_test( NAME ${PREFIX}/${TEST_CASE}/trace-0.jsonl COMMAND ${CMAKE_COMMAND} -E cat - ${CMAKE_CURRENT_BINARY_DIR}/${TEST_CASE}/trace-0-0x03141f8608bc78c8b8fb23275febeb6bf40bd348bd68b703a92bf2ce8ac8bdc0.jsonl + ${CMAKE_CURRENT_BINARY_DIR}/${TEST_CASE}/trace-0-0x2ea18c1067829310d4eae2d89bbd2200d2f525d529484d46f53d5b42a53260f8.jsonl ) set_tests_properties( ${PREFIX}/${TEST_CASE}/trace-0.jsonl PROPERTIES @@ -319,7 +319,7 @@ set_tests_properties( # The whole encoding: an empty transaction list is hex too, and is what a run which never read # --input.txs writes. PASS_REGULAR_EXPRESSION - "^0xf84ef84c8032830186a08080801ba0468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665aea05cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "^0xf84ef84c8032830186a08080801ba0118e6dc7beddd8c4fe1c2daa896c030f7cf7983b244113bf358d239f615ae084a02eee90a3198ef1d59e390db04dd385bd1cd27bddcacbf843c17a7dd46bc9700b" ) diff --git a/test/integration/evmone-cli/t8n/cancun_create_tx/txs.json b/test/integration/evmone-cli/t8n/cancun_create_tx/txs.json index e8fc3c6846..2d2bb9f36e 100644 --- a/test/integration/evmone-cli/t8n/cancun_create_tx/txs.json +++ b/test/integration/evmone-cli/t8n/cancun_create_tx/txs.json @@ -9,8 +9,8 @@ "chainId": "0x1", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", "v": "0x1b", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "r": "0x5b3bfaf21a50f2d82a624d2220b89f38631e0aa30be189a5aa0e4c63a3b9fa3d", + "s": "0x7001087f3f261f606995bea6483b8aec67b8c654213443904750b446723fa35d" }, { "input": "0x00", diff --git a/test/integration/evmone-cli/t8n/osaka_blob_schedule/txs.json b/test/integration/evmone-cli/t8n/osaka_blob_schedule/txs.json index 1d64142aff..b8e25240f3 100644 --- a/test/integration/evmone-cli/t8n/osaka_blob_schedule/txs.json +++ b/test/integration/evmone-cli/t8n/osaka_blob_schedule/txs.json @@ -9,7 +9,7 @@ "chainId": "0x1", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", "v": "0x1b", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "r": "0x118e6dc7beddd8c4fe1c2daa896c030f7cf7983b244113bf358d239f615ae084", + "s": "0x2eee90a3198ef1d59e390db04dd385bd1cd27bddcacbf843c17a7dd46bc9700b" } ] diff --git a/test/integration/evmone-cli/t8n/prague_empty_requests/txs.json b/test/integration/evmone-cli/t8n/prague_empty_requests/txs.json index 1d64142aff..b8e25240f3 100644 --- a/test/integration/evmone-cli/t8n/prague_empty_requests/txs.json +++ b/test/integration/evmone-cli/t8n/prague_empty_requests/txs.json @@ -9,7 +9,7 @@ "chainId": "0x1", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", "v": "0x1b", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "r": "0x118e6dc7beddd8c4fe1c2daa896c030f7cf7983b244113bf358d239f615ae084", + "s": "0x2eee90a3198ef1d59e390db04dd385bd1cd27bddcacbf843c17a7dd46bc9700b" } ] diff --git a/test/integration/evmone-cli/t8n/prague_pre_state_only/txs.json b/test/integration/evmone-cli/t8n/prague_pre_state_only/txs.json index 1d64142aff..b8e25240f3 100644 --- a/test/integration/evmone-cli/t8n/prague_pre_state_only/txs.json +++ b/test/integration/evmone-cli/t8n/prague_pre_state_only/txs.json @@ -9,7 +9,7 @@ "chainId": "0x1", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", "v": "0x1b", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "r": "0x118e6dc7beddd8c4fe1c2daa896c030f7cf7983b244113bf358d239f615ae084", + "s": "0x2eee90a3198ef1d59e390db04dd385bd1cd27bddcacbf843c17a7dd46bc9700b" } ] diff --git a/test/state/transaction.cpp b/test/state/transaction.cpp index b528b9449a..0c28b518ee 100644 --- a/test/state/transaction.cpp +++ b/test/state/transaction.cpp @@ -145,6 +145,11 @@ std::optional
recover_sender(const Transaction& tx, bytes_view txbytes) { // The signing preimage is the transaction's encoding without the trailing (v, r, s). const auto typed = tx.type != Transaction::Type::legacy; + + // The decoder bounds v, but a transaction built from JSON carries whatever v the input names. + if (typed ? tx.v > 1 : (tx.v != 27 && tx.v != 28 && tx.v < 35)) + return std::nullopt; + auto envelope = txbytes.substr(typed ? 1 : 0); // Skip the EIP-2718 type byte. bytes_view payload; [[maybe_unused]] const auto is_list = rlp::take_list_payload(envelope, payload); diff --git a/test/state/transaction.hpp b/test/state/transaction.hpp index 70879aec1c..3e0f3185ec 100644 --- a/test/state/transaction.hpp +++ b/test/state/transaction.hpp @@ -88,13 +88,14 @@ struct Transaction /// Handles the legacy RLP list and the EIP-2718 typed envelope (type byte followed by an RLP list). [[nodiscard]] std::optional decode_transaction(bytes_view data) noexcept; -/// Recovers the sender (the signer) of the transaction @p tx decoded from @p txbytes, +/// Recovers the sender (the signer) of the transaction @p tx serialized as @p txbytes, /// or std::nullopt if the signature is invalid. /// /// The serialization is needed as well because the signing preimage is a slice of it; @p tx must -/// be what decode_transaction(@p txbytes) returned. +/// be what decode_transaction(@p txbytes) would return. /// -/// The recovery is strict at every revision: r, s in [1, secp256k1n) and low s (EIP-2). +/// The recovery is strict at every revision: v valid for its type, r, s in [1, secp256k1n) +/// and low s (EIP-2). [[nodiscard]] std::optional
recover_sender( const Transaction& tx, bytes_view txbytes) noexcept; diff --git a/test/unittests/tooling_t8n_test.cpp b/test/unittests/tooling_t8n_test.cpp index 0f3b60c316..b512973ad4 100644 --- a/test/unittests/tooling_t8n_test.cpp +++ b/test/unittests/tooling_t8n_test.cpp @@ -39,8 +39,8 @@ constexpr auto ALLOC_JSON = R"({ // Single legacy CREATE transaction; init code is `PUSH1 0x01 PUSH0 RETURN`, // which deploys a one-byte runtime `0x00`. Three opcodes => three trace lines. -// Matches test/integration/evmone-cli/t8n/cancun_create_tx/txs.json[0]; the tx hash is -// well-known and used below. +// Matches test/integration/evmone-cli/t8n/cancun_create_tx/txs.json[0]. Signed by the account +// ALLOC_JSON funds; t8n ignores `sender`. constexpr auto TX_JSON = R"([{ "to": null, "input": "0x60015ff3", @@ -51,8 +51,8 @@ constexpr auto TX_JSON = R"([{ "chainId": "0x1", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", "v": "0x1b", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "r": "0x5b3bfaf21a50f2d82a624d2220b89f38631e0aa30be189a5aa0e4c63a3b9fa3d", + "s": "0x7001087f3f261f606995bea6483b8aec67b8c654213443904750b446723fa35d" }])"; /// Runs t8n over the given pre-state and transactions, and returns the result JSON. @@ -77,8 +77,7 @@ std::string run_t8n(std::string_view alloc_json, std::string_view txs_json, evmc return out_result.str(); } -/// Legacy transaction calling CALLEE. t8n takes `sender` from the JSON and only checks `hash` -/// when present, so the signature is never recovered. +/// Legacy transaction calling CALLEE, signed like TX_JSON. constexpr auto TX_TO_CALLEE = R"([{ "to": "0x000000000000000000000000000000000000c0de", "input": "0x", @@ -88,11 +87,22 @@ constexpr auto TX_TO_CALLEE = R"([{ "gasPrice": "0x32", "chainId": "0x1", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", - "v": "0x1b", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "v": "0x1c", + "r": "0x1059a4656fdf5558d07e5e1e0734e9002f4946f727b45aa52907f76f220735ce", + "s": "0x7bf197eb7303fde325d7c24dda50315f155814df4716e1e611ae0e3460231045" }])"; +/// Runs t8n over ALLOC_JSON and returns the error of the one transaction it must reject. +std::string rejection(std::string_view txs_json, evmc_revision rev) +{ + const auto result = json::parse(run_t8n(ALLOC_JSON, txs_json, rev)); + EXPECT_EQ(result.at("receipts"), json::array()); + EXPECT_EQ(result.at("rejected").size(), 1u); + if (result.at("rejected").empty()) + return {}; + return result.at("rejected")[0].at("error"); +} + /// Runs TX_TO_CALLEE against a callee deployed with the given code. std::string run_call_to(std::string_view callee_code, evmc_revision rev) { @@ -196,8 +206,8 @@ TEST(tooling_t8n, blob_transaction_creating_a_contract_is_rejected) "nonce": "0x0", "value": "0x0", "v": "0x0", - "r": "0xfc12b67159a3567f8bdbc49e0be369a2e20e09d57a51c41310543a4128409464", - "s": "0x2de0cfe5495c4f58ff60645ceda0afd67a4c90a70bc89fe207269435b35e5b67", + "r": "0x78ce8eb2b34bb6cce4cedfb5249c29467270c41450fa4a2439797804292cfba6", + "s": "0x5437b757e8094f62ca2a3e3a8696cfbd531620bc3c337368db36d0829e073004", "maxFeePerGas": "0x32", "maxPriorityFeePerGas": "0x2", "maxFeePerBlobGas": "0xa", @@ -207,11 +217,86 @@ TEST(tooling_t8n, blob_transaction_creating_a_contract_is_rejected) "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b" }])"; - const auto result = json::parse(run_t8n(ALLOC_JSON, BLOB_CREATE_TX, EVMC_CANCUN)); - EXPECT_EQ(result.at("receipts"), json::array()); - ASSERT_EQ(result.at("rejected").size(), 1u); EXPECT_EQ( - result.at("rejected")[0].at("error"), "TransactionException.TYPE_3_TX_CONTRACT_CREATION"); + rejection(BLOB_CREATE_TX, EVMC_CANCUN), "TransactionException.TYPE_3_TX_CONTRACT_CREATION"); +} + +TEST(tooling_t8n, unrecoverable_signature_is_rejected) +{ + // r = 5 is in range yet no curve point has x = 5 (5**3 + 7 is a quadratic non-residue), so + // only the recovery fails. `sender` names the funded account, so taking it instead of the + // signature would execute the transaction. + static constexpr auto UNRECOVERABLE_TX = R"([{ + "to": "0x000000000000000000000000000000000000c0de", + "input": "0x", + "gas": "0x186a0", + "nonce": "0x0", + "value": "0x1", + "gasPrice": "0x32", + "chainId": "0x1", + "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", + "v": "0x1b", + "r": "0x5", + "s": "0x1" + }])"; + + EXPECT_EQ( + rejection(UNRECOVERABLE_TX, EVMC_SHANGHAI), "TransactionException.INVALID_SIGNATURE_VRS"); +} + +TEST(tooling_t8n, transaction_with_an_impossible_v_is_rejected) +{ + // 34 is neither 27, 28 nor EIP-155's 35+; 2 is not a y parity. + static constexpr auto LEGACY_BAD_V = R"([{ + "to": "0x000000000000000000000000000000000000c0de", + "input": "0x", + "gas": "0x186a0", + "nonce": "0x0", + "value": "0x0", + "gasPrice": "0x32", + "chainId": "0x1", + "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", + "v": "0x22", + "r": "0x1", + "s": "0x1" + }])"; + static constexpr auto TYPED_BAD_V = R"([{ + "to": "0x000000000000000000000000000000000000c0de", + "input": "0x", + "gas": "0x186a0", + "nonce": "0x0", + "value": "0x0", + "maxFeePerGas": "0x32", + "maxPriorityFeePerGas": "0x2", + "chainId": "0x1", + "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", + "v": "0x2", + "r": "0x1", + "s": "0x1" + }])"; + + EXPECT_EQ(rejection(LEGACY_BAD_V, EVMC_SHANGHAI), "TransactionException.INVALID_SIGNATURE_VRS"); + EXPECT_EQ(rejection(TYPED_BAD_V, EVMC_SHANGHAI), "TransactionException.INVALID_SIGNATURE_VRS"); +} + +TEST(tooling_t8n, transaction_signed_for_another_chain_is_rejected) +{ + // Signed for chain 2 (v = 35 + 2 * 2 + 0), run on chain 1. + static constexpr auto TX_FOR_CHAIN_2 = R"([{ + "to": null, + "input": "0x60015ff3", + "gas": "0x186a0", + "nonce": "0x0", + "value": "0x0", + "gasPrice": "0x32", + "chainId": "0x2", + "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", + "v": "0x27", + "r": "0xf38f7d563c747b90705401b348a87bc0fbd94def6b31675f09ef74539504050a", + "s": "0x4c6b5cce6e4a4cd2c70e5d41dffab7a9c067f292b4c9b827487ed04ba7d175c6" + }])"; + + EXPECT_EQ(rejection(TX_FOR_CHAIN_2, EVMC_SHANGHAI), "TransactionException.INVALID_CHAINID"); } TEST(tooling_t8n, a_block_requesting_nothing_reports_the_empty_requests_hash) @@ -372,8 +457,8 @@ TEST(tooling_t8n, mismatched_tx_hash_throws) "chainId": "0x1", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", "v": "0x1b", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc", + "r": "0x5b3bfaf21a50f2d82a624d2220b89f38631e0aa30be189a5aa0e4c63a3b9fa3d", + "s": "0x7001087f3f261f606995bea6483b8aec67b8c654213443904750b446723fa35d", "hash": "0xdeadbeef00000000000000000000000000000000000000000000000000000000" }])"; @@ -408,8 +493,8 @@ TEST(tooling_t8n, max_chain_id) "chainId": "0xffffffffffffffff", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", "v": "0x1b", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "r": "0x5b3bfaf21a50f2d82a624d2220b89f38631e0aa30be189a5aa0e4c63a3b9fa3d", + "s": "0x7001087f3f261f606995bea6483b8aec67b8c654213443904750b446723fa35d" }])"; std::istringstream env{ENV_JSON}; @@ -437,18 +522,20 @@ TEST(tooling_t8n, max_v) // The maximum `v` (uint64 max = 0xffffffffffffffff) must be parsed and executed without // overflow; regression test for `v` being loaded as `uint8_t`, which threw // `from_json: value > 0xFF`. + // Only chain id 0x7fffffffffffffee with y parity 0 produces such a `v`, so the transaction + // is signed for that chain; its gas is 0x186a1 because that preimage signs with parity 0. static constexpr auto TX_MAX_V = R"([{ "to": null, "input": "0x60015ff3", - "gas": "0x186a0", + "gas": "0x186a1", "nonce": "0x0", "value": "0x0", "gasPrice": "0x32", - "chainId": "0x1", + "chainId": "0x7fffffffffffffee", "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b", "v": "0xffffffffffffffff", - "r": "0x468a915f087692bb9be503831a3dfef2cf9c8dee26deb40ff2ec99e8d22665ae", - "s": "0x5cedae0810c3851ecd1004bfdbfe6ddc7753c2d665993bb01ce75af7857b13dc" + "r": "0x463a1ed30b21771a94e83a0ded54cce75850152b18dcd13a1a068d31a5599270", + "s": "0x4f1a132cab35ed35cebe2512004572b1e2bede650ae75a3923be03327ae600c2" }])"; std::istringstream env{ENV_JSON}; @@ -458,7 +545,7 @@ TEST(tooling_t8n, max_v) tooling::T8NArgs args; args.rev = EVMC_SHANGHAI; - args.chain_id = 1; + args.chain_id = 0x7fffffffffffffee; args.alloc = &alloc; args.env = &env; args.txs = &txs; diff --git a/test/utils/block_transition.cpp b/test/utils/block_transition.cpp index c8a36a733b..8e56d467e6 100644 --- a/test/utils/block_transition.cpp +++ b/test/utils/block_transition.cpp @@ -55,8 +55,21 @@ TransitionResult apply_block(const TestState& state, evmc::VM& vm, const state:: for (size_t i = 0; i < txs.size(); ++i) { - const auto& tx = txs[i]; - const auto computed_tx_hash = keccak256(rlp::encode(tx)); + auto tx = txs[i]; + const auto txbytes = rlp::encode(tx); + const auto computed_tx_hash = keccak256(txbytes); + + if (opts.recover_senders) + { + const auto sender = state::recover_sender(tx, txbytes); + if (!sender.has_value()) + { + rejected_txs.push_back( + {computed_tx_hash, i, make_error_code(state::INVALID_SIGNATURE)}); + continue; + } + tx.sender = *sender; + } std::optional trace_guard; if (trace_enabled) diff --git a/test/utils/block_transition.hpp b/test/utils/block_transition.hpp index c95628ac47..91f0a796f0 100644 --- a/test/utils/block_transition.hpp +++ b/test/utils/block_transition.hpp @@ -39,6 +39,10 @@ struct BlockTransitionOptions /// (t8n pre-state-only mode). The transaction loop and finalization still run. bool skip_system_calls = false; + /// Recover each transaction's sender from its signature instead of taking the one the + /// input names; a signature recovering nothing rejects the transaction. + bool recover_senders = true; + /// Called once per transaction (just before execution) to obtain a per-tx /// trace sink; std::clog is redirected to the returned stream for the /// duration of that transaction. Unset = tracing disabled. diff --git a/test/utils/blockchaintest_runner.cpp b/test/utils/blockchaintest_runner.cpp index d97c7c36f6..891ce160d8 100644 --- a/test/utils/blockchaintest_runner.cpp +++ b/test/utils/blockchaintest_runner.cpp @@ -292,7 +292,7 @@ void run_blockchain_test(const BlockchainTest& test, evmc::VM& vm, TestReport& r const auto& pre_state = parent_data_it->second.post_state; auto res = apply_block(pre_state, vm, bi, block_hashes, test_block.transactions, rev, - blob_gas_limit, {.block_reward = mining_reward(rev)}); + blob_gas_limit, {.block_reward = mining_reward(rev), .recover_senders = false}); if (res.requests_error) { @@ -379,8 +379,9 @@ void run_blockchain_test(const BlockchainTest& test, evmc::VM& vm, TestReport& r const auto sender_not_recovered = contains_any( test_block.expected_exception, "TransactionException.INVALID_SIGNATURE_VRS"); - const auto res = apply_block(pre_state, vm, bi, block_hashes, test_block.transactions, - rev, blob_gas_limit, {.block_reward = mining_reward(rev)}); + const auto res = + apply_block(pre_state, vm, bi, block_hashes, test_block.transactions, rev, + blob_gas_limit, {.block_reward = mining_reward(rev), .recover_senders = false}); if (!res.rejected.empty()) { // A transaction was rejected: the fixture must name that reason, not merely diff --git a/test/utils/t8n.cpp b/test/utils/t8n.cpp index ff5096d282..7b3061b191 100644 --- a/test/utils/t8n.cpp +++ b/test/utils/t8n.cpp @@ -91,8 +91,8 @@ void t8n(evmc::VM& vm, const T8NArgs& args) j_result["receipts"] = JSON::array(); j_result["rejected"] = JSON::array(); - // Parse the transactions, assign the chain ID and validate any provided hash. A non-array - // `txs` value yields zero transactions but still produces a full, finalized block result. + // Parse the transactions and validate any provided hash. A non-array `txs` value yields + // zero transactions but still produces a full, finalized block result. std::vector txs; if (j_txs.is_array()) { @@ -100,7 +100,12 @@ void t8n(evmc::VM& vm, const T8NArgs& args) for (const auto& j_tx : j_txs) { auto tx = from_json(j_tx); - tx.chain_id = args.chain_id; + + // A legacy transaction's chain id is the one its v encodes (EIP-155), as + // decode_transaction() reads it; taking --state.chainid made the chain id + // check vacuous. + if (tx.type == state::Transaction::Type::legacy) + tx.chain_id = tx.v >= 35 ? (tx.v - 35) / 2 : 0; if (const auto loaded_tx_hash = load_optional(j_tx, "hash")) {