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.

Sign up for GitHub

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

statetest: Reuse Transaction loading in TestMultiTransaction #556

Merged
merged 4 commits into from
Feb 14, 2023
Merged

statetest: Reuse Transaction loading in TestMultiTransaction #556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
50c2108
test: Add transaction JSON loading unit tests
chfast Feb 8, 2023
f43158f
statetest: Move from_json<Transaction> implementation
chfast Feb 8, 2023
b4ab2f2
statetest: Reuse Transaction loading in TestMultiTransaction
chfast Feb 8, 2023
8749c51
statetest: Error on parsing Transaction with mixed legacy/1559 contents
axic Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 34 additions & 37 deletions test/statetest/statetest_loader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,23 +182,51 @@ evmc_revision to_rev(std::string_view s)
throw std::invalid_argument{"unknown revision: " + std::string{s}};
}

static void from_json(const json::json& j, TestMultiTransaction& o)
/// Load common parts of Transaction or TestMultiTransaction.
static void from_json_tx_common(const json::json& j, state::Transaction& o)
{
if (j.contains("gasPrice"))
o.sender = from_json<evmc::address>(j.at("sender"));

if (const auto& to = j.at("to"); !to.get<std::string>().empty())
o.to = from_json<evmc::address>(to);

if (const auto gas_price_it = j.find("gasPrice"); gas_price_it != j.end())
{
o.kind = state::Transaction::Kind::legacy;
o.max_gas_price = from_json<intx::uint256>(j.at("gasPrice"));
o.max_gas_price = from_json<intx::uint256>(*gas_price_it);
o.max_priority_gas_price = o.max_gas_price;
if (j.contains("maxFeePerGas") || j.contains("maxPriorityFeePerGas"))
{
throw std::invalid_argument(
Copy link
Member

Choose a reason for hiding this comment

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

This could be in braces {}, because multi-line.

Copy link
Member

Choose a reason for hiding this comment

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

Changed.

"Misformatted transaction -- contains both legacy and 1559 fees");
}
}
else
{
o.kind = state::Transaction::Kind::eip1559;
o.max_gas_price = from_json<intx::uint256>(j.at("maxFeePerGas"));
o.max_priority_gas_price = from_json<intx::uint256>(j.at("maxPriorityFeePerGas"));
}
o.sender = from_json<evmc::address>(j.at("sender"));
if (!j.at("to").get<std::string>().empty())
o.to = from_json<evmc::address>(j["to"]);
}

template <>
state::Transaction from_json<state::Transaction>(const json::json& j)
{
state::Transaction o;
from_json_tx_common(j, o);
o.data = from_json<bytes>(j.at("input"));
o.gas_limit = from_json<int64_t>(j.at("gas"));
o.value = from_json<intx::uint256>(j.at("value"));

if (const auto ac_it = j.find("accessList"); ac_it != j.end())
o.access_list = from_json<state::AccessList>(*ac_it);

return o;
}

static void from_json(const json::json& j, TestMultiTransaction& o)
{
from_json_tx_common(j, o);

for (const auto& j_data : j.at("data"))
o.inputs.emplace_back(from_json<bytes>(j_data));
Expand Down Expand Up @@ -255,37 +283,6 @@ static void from_json(const json::json& j, StateTransitionTest& o)
}
}

template <>
state::Transaction from_json<state::Transaction>(const json::json& j)
{
state::Transaction o;
o.data = from_json<bytes>(j.at("input"));
o.gas_limit = from_json<int64_t>(j.at("gas"));
o.value = from_json<intx::uint256>(j.at("value"));
o.sender = from_json<evmc::address>(j.at("sender"));

if (!j.at("to").get<std::string>().empty())
o.to = from_json<evmc::address>(j.at("to"));

if (j.contains("gasPrice"))
{
o.kind = state::Transaction::Kind::legacy;
o.max_gas_price = from_json<intx::uint256>(j.at("gasPrice"));
o.max_priority_gas_price = o.max_gas_price;
}
else
{
o.kind = state::Transaction::Kind::eip1559;
o.max_gas_price = from_json<intx::uint256>(j.at("maxFeePerGas"));
o.max_priority_gas_price = from_json<intx::uint256>(j.at("maxPriorityFeePerGas"));
}

if (j.contains("accessList"))
o.access_list = from_json<state::AccessList>(j.at("accessList"));

return o;
}

StateTransitionTest load_state_test(const fs::path& test_file)
{
return json::json::parse(std::ifstream{test_file}).get<StateTransitionTest>();
Expand Down
1 change: 1 addition & 0 deletions test/unittests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ add_executable(evmone-unittests
state_mpt_hash_test.cpp
state_mpt_test.cpp
state_rlp_test.cpp
statetest_loader_tx_test.cpp
statetest_logs_hash_test.cpp
tracing_test.cpp
utils_test.cpp
Expand Down
106 changes: 106 additions & 0 deletions test/unittests/statetest_loader_tx_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// evmone: Fast Ethereum Virtual Machine implementation
// Copyright 2023 The evmone Authors.
// SPDX-License-Identifier: Apache-2.0

#include <gtest/gtest.h>
#include <test/statetest/statetest.hpp>

using namespace evmone;

TEST(statetest_loader, tx_create_legacy)
Copy link
Member

Choose a reason for hiding this comment

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

Should probably add a test where both gasPrice and maxFeePerGas is present.

Copy link
Member Author

Choose a reason for hiding this comment

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

What should happen then?

Copy link
Member

Choose a reason for hiding this comment

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

Just fail, pushed my version.

{
constexpr std::string_view input = R"({
"input": "b0b1",
"gas": "9091",
"value": "0xe0e1",
"sender": "a0a1",
"to": "",
"gasPrice": "0x7071"
})";

const auto tx = test::from_json<state::Transaction>(json::json::parse(input));
EXPECT_EQ(tx.kind, state::Transaction::Kind::legacy);
EXPECT_EQ(tx.data, (bytes{0xb0, 0xb1}));
EXPECT_EQ(tx.gas_limit, 0x9091);
EXPECT_EQ(tx.value, 0xe0e1);
EXPECT_EQ(tx.sender, 0xa0a1_address);
EXPECT_FALSE(tx.to.has_value());
EXPECT_EQ(tx.max_gas_price, 0x7071);
EXPECT_EQ(tx.max_priority_gas_price, 0x7071);
Copy link
Member

Choose a reason for hiding this comment

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

Hm, are these correct? Are they both set to gasPrice? I thought the logic is more complex.

Copy link
Member Author

Choose a reason for hiding this comment

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

It turns out you can you EIP-1559 logic this way also for legacy transactions.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the logic is as simple as that in https://eips.ethereum.org/EIPS/eip-1559:

	def normalize_transaction(self, transaction: Transaction, signer_address: int) -> NormalizedTransaction:
		# legacy transactions
		if isinstance(transaction, TransactionLegacy):
			return NormalizedTransaction(
				signer_address = signer_address,
				signer_nonce = transaction.signer_nonce,
				gas_limit = transaction.gas_limit,
				max_priority_fee_per_gas = transaction.gas_price,
				max_fee_per_gas = transaction.gas_price,
				destination = transaction.destination,
				amount = transaction.amount,
				payload = transaction.payload,
				access_list = [],
			)

EXPECT_TRUE(tx.access_list.empty());
}

TEST(statetest_loader, tx_eip1559)
{
constexpr std::string_view input = R"({
"input": "b0b1",
"gas": "9091",
"value": "0xe0e1",
"sender": "a0a1",
"to": "c0c1",
"maxFeePerGas": "0x7071",
"maxPriorityFeePerGas": "0x6061",
"accessList": []
})";

const auto tx = test::from_json<state::Transaction>(json::json::parse(input));
EXPECT_EQ(tx.kind, state::Transaction::Kind::eip1559);
EXPECT_EQ(tx.data, (bytes{0xb0, 0xb1}));
EXPECT_EQ(tx.gas_limit, 0x9091);
EXPECT_EQ(tx.value, 0xe0e1);
EXPECT_EQ(tx.sender, 0xa0a1_address);
EXPECT_EQ(tx.to, 0xc0c1_address);
EXPECT_EQ(tx.max_gas_price, 0x7071);
EXPECT_EQ(tx.max_priority_gas_price, 0x6061);
EXPECT_TRUE(tx.access_list.empty());
}

TEST(statetest_loader, tx_access_list)
{
constexpr std::string_view input = R"({
"input": "",
"gas": "0",
"value": "0",
"sender": "",
"to": "",
"maxFeePerGas": "0",
"maxPriorityFeePerGas": "0",
"accessList": [
{"address": "ac01", "storageKeys": []},
{"address": "ac02", "storageKeys": ["fe", "00"]}
]
})";

const auto tx = test::from_json<state::Transaction>(json::json::parse(input));
Copy link
Member

Choose a reason for hiding this comment

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

Technically could add the checks for each of the zero values.

Copy link
Member

Choose a reason for hiding this comment

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

Extended tests.

EXPECT_EQ(tx.kind, state::Transaction::Kind::eip1559);
EXPECT_TRUE(tx.data.empty());
EXPECT_EQ(tx.gas_limit, 0);
EXPECT_EQ(tx.value, 0);
EXPECT_EQ(tx.sender, address{}); // TODO: use 0x0_address?
EXPECT_FALSE(tx.to.has_value());
EXPECT_EQ(tx.max_gas_price, 0);
EXPECT_EQ(tx.max_priority_gas_price, 0);
ASSERT_EQ(tx.access_list.size(), 2);
EXPECT_EQ(tx.access_list[0].first, 0xac01_address);
EXPECT_EQ(tx.access_list[0].second.size(), 0);
EXPECT_EQ(tx.access_list[1].first, 0xac02_address);
EXPECT_EQ(tx.access_list[1].second, (std::vector{0xfe_bytes32, 0x00_bytes32}));
}

TEST(statetest_loader, tx_confusing)
{
constexpr std::string_view input = R"({
"input": "b0b1",
"gas": "9091",
"value": "0xe0e1",
"sender": "a0a1",
"to": "c0c1",
"gasPrice": "0x8081",
"maxFeePerGas": "0x7071",
"maxPriorityFeePerGas": "0x6061",
"accessList": []
})";

EXPECT_THROW(
test::from_json<state::Transaction>(json::json::parse(input)), std::invalid_argument);
}