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

[0.5] Enforce chain-id in transactions pushed to the contract #620

Merged
merged 2 commits into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion contract/include/evm_runtime/evm_contract.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class [[eosio::contract]] evm_contract : public contract
check((_config.get().status & static_cast<uint32_t>(status_flags::frozen)) == 0, "contract is frozen");
}

silkworm::Receipt execute_tx(eosio::name miner, silkworm::Block& block, silkworm::Transaction& tx, silkworm::ExecutionProcessor& ep);
silkworm::Receipt execute_tx(eosio::name miner, silkworm::Block& block, silkworm::Transaction& tx, silkworm::ExecutionProcessor& ep, bool enforce_chain_id);

uint64_t get_and_increment_nonce(const name owner);

Expand Down
10 changes: 7 additions & 3 deletions contract/src/actions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ void check_result( ValidationResult r, const Transaction& txn, const char* desc
eosio::check( false, std::move(err_msg));
}

Receipt evm_contract::execute_tx( eosio::name miner, Block& block, Transaction& tx, silkworm::ExecutionProcessor& ep ) {
Receipt evm_contract::execute_tx( eosio::name miner, Block& block, Transaction& tx, silkworm::ExecutionProcessor& ep, bool enforce_chain_id) {
//when being called as an inline action, clutch in allowance for reserved addresses & signatures by setting from_self=true
const bool from_self = get_sender() == get_self();

Expand Down Expand Up @@ -244,6 +244,10 @@ Receipt evm_contract::execute_tx( eosio::name miner, Block& block, Transaction&
else if(is_reserved_address(*tx.from))
check(from_self, "bridge signature used outside of bridge transaction");

if(enforce_chain_id && !from_self) {
check(tx.chain_id.has_value(), "tx without chain-id");
}

ValidationResult r = consensus::pre_validate_transaction(tx, ep.evm().block().header.number, ep.evm().config(),
ep.evm().block().header.base_fee_per_gas);
check_result( r, tx, "pre_validate_transaction error" );
Expand Down Expand Up @@ -402,7 +406,7 @@ void evm_contract::pushtx( eosio::name miner, const bytes& rlptx ) {
check(tx.max_priority_fee_per_gas == tx.max_fee_per_gas, "max_priority_fee_per_gas must be equal to max_fee_per_gas");
check(tx.max_fee_per_gas >= current_config.gas_price, "gas price is too low");

auto receipt = execute_tx(miner, block, tx, ep);
auto receipt = execute_tx(miner, block, tx, ep, true);

engine.finalize(ep.state(), ep.evm().block(), ep.evm().revision());
ep.state().write_to_db(ep.evm().block().header.number);
Expand Down Expand Up @@ -569,7 +573,7 @@ bool evm_contract::gc(uint32_t max) {
ByteView bv{(const uint8_t*)orlptx->data(), orlptx->size()};
eosio::check(rlp::decode(bv,tx) == DecodingResult::kOk && bv.empty(), "unable to decode transaction");

execute_tx(eosio::name{}, block, tx, ep);
execute_tx(eosio::name{}, block, tx, ep, false);
}
engine.finalize(ep.state(), ep.evm().block(), ep.evm().revision());
ep.state().write_to_db(ep.evm().block().header.number);
Expand Down
1 change: 1 addition & 0 deletions contract/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ add_eosio_test_executable( unit_test
${CMAKE_SOURCE_DIR}/gas_fee_tests.cpp
${CMAKE_SOURCE_DIR}/blockhash_tests.cpp
${CMAKE_SOURCE_DIR}/exec_tests.cpp
${CMAKE_SOURCE_DIR}/chainid_tests.cpp
${CMAKE_SOURCE_DIR}/main.cpp
${CMAKE_SOURCE_DIR}/silkworm/core/silkworm/rlp/encode.cpp
${CMAKE_SOURCE_DIR}/silkworm/core/silkworm/rlp/decode.cpp
Expand Down
9 changes: 7 additions & 2 deletions contract/tests/basic_evm_tester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,15 @@ key256_t evm_eoa::address_key256() const
return fixed_bytes<32>(buffer).get_array();
}

void evm_eoa::sign(silkworm::Transaction& trx)
void evm_eoa::sign(silkworm::Transaction& trx) {
sign(trx, basic_evm_tester::evm_chain_id);
}

void evm_eoa::sign(silkworm::Transaction& trx, std::optional<uint64_t> evm_chain_id)
{
silkworm::Bytes rlp;
trx.chain_id = basic_evm_tester::evm_chain_id;
if(evm_chain_id.has_value())
trx.chain_id = evm_chain_id.value();
trx.nonce = next_nonce++;
silkworm::rlp::encode(rlp, trx, true, false);
ethash::hash256 hash{silkworm::keccak256(rlp)};
Expand Down
1 change: 1 addition & 0 deletions contract/tests/basic_evm_tester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class evm_eoa
key256_t address_key256() const;

void sign(silkworm::Transaction& trx);
void sign(silkworm::Transaction& trx, std::optional<uint64_t> chain_id);

~evm_eoa();

Expand Down
42 changes: 42 additions & 0 deletions contract/tests/chainid_tests.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#include "basic_evm_tester.hpp"
#include <silkworm/execution/address.hpp>

using namespace evm_test;
struct chain_id_tester : basic_evm_tester {
chain_id_tester() {
create_accounts({"alice"_n});
transfer_token(faucet_account_name, "alice"_n, make_asset(10000'0000));
init();
}
};

BOOST_AUTO_TEST_SUITE(chainid_evm_tests)
BOOST_FIXTURE_TEST_CASE(chainid_tests, chain_id_tester) try {

// Bridge transactions are sent without chain id specified (must succeed)
evm_eoa evm1;
const int64_t to_bridge = 1000000; //100.000 EOS
transfer_token("alice"_n, "evm"_n, make_asset(to_bridge), evm1.address_0x());
auto balance_evm1 = evm_balance(evm1);
BOOST_REQUIRE(balance_evm1.has_value() && *balance_evm1 == 100_ether);

// Transactions without chain id specified must fail
evm_eoa evm2;
auto txn = generate_tx(evm2.address, 1_ether, 21000);
evm1.sign(txn, {});
BOOST_REQUIRE_EXCEPTION(pushtx(txn), eosio_assert_message_exception,
[](const eosio_assert_message_exception& e) {return testing::expect_assert_message(e, "assertion failure with message: tx without chain-id");});

// reuse nonce since previous signed transaction was rejected
evm1.next_nonce--;

// Transactions with chain id must succeed
evm_eoa evm3;
txn = generate_tx(evm3.address, 20_ether, 21000);
evm1.sign(txn, basic_evm_tester::evm_chain_id);
pushtx(txn);
auto balance_evm3 = evm_balance(evm3);
BOOST_REQUIRE(balance_evm3.has_value() && *balance_evm3 == 20_ether);

} FC_LOG_AND_RETHROW()
BOOST_AUTO_TEST_SUITE_END()