diff --git a/category/execution/ethereum/test/test_monad_chain.cpp b/category/execution/ethereum/test/test_monad_chain.cpp index 7490139a9..159be4496 100644 --- a/category/execution/ethereum/test/test_monad_chain.cpp +++ b/category/execution/ethereum/test/test_monad_chain.cpp @@ -156,8 +156,8 @@ void run_revert_transaction_test( uint8_t const prevent_dip_bitset, uint64_t const initial_balance_mon, uint64_t const gas_fee_mon, uint64_t const value_mon, bool const expected) { - constexpr uint256_t BASE_FEE_PER_GAS = 10; - constexpr Address SENDER{1}; + static constexpr uint256_t BASE_FEE_PER_GAS = 10; + static constexpr Address SENDER{1}; InMemoryMachine machine; mpt::Db db{machine}; TrieDb tdb{db}; @@ -347,6 +347,83 @@ TYPED_TEST(MonadTraitsTest, reserve_balance_checks_disabled_before_monad_four) } } +TYPED_TEST( + MonadTraitsTest, + sender_gas_fee_above_reserve_stays_failed_after_large_credit) +{ + using traits = typename TestFixture::Trait; + if constexpr (traits::monad_rev() < MONAD_FOUR) { + GTEST_SKIP() << "reserve-balance checks are disabled before MONAD_FOUR"; + } + + static constexpr Address SENDER{1}; + static constexpr uint256_t BASE_FEE_PER_GAS = 10; + auto const to_wei = [](uint64_t mon) { + return uint256_t{mon} * 1000000000000000000ULL; + }; + + InMemoryMachine machine; + mpt::Db db{machine}; + TrieDb tdb{db}; + vm::VM vm; + BlockState bs{tdb, vm}; + + { + State init_state{bs, Incarnation{0, 0}}; + init_state.add_to_balance(SENDER, to_wei(20)); + MONAD_ASSERT(bs.can_merge(init_state)); + bs.merge(init_state); + } + + uint256_t const sender_gas_fee = to_wei(11); // reserve is capped at 10 MON + uint256_t const gas_limit_u256 = sender_gas_fee / BASE_FEE_PER_GAS; + MONAD_ASSERT( + (sender_gas_fee % BASE_FEE_PER_GAS) == 0 && + gas_limit_u256 <= std::numeric_limits::max()); + + Transaction const tx{ + .max_fee_per_gas = BASE_FEE_PER_GAS, + .gas_limit = static_cast(gas_limit_u256), + .type = TransactionType::legacy, + .max_priority_fee_per_gas = 0, + }; + + ankerl::unordered_dense::segmented_set
const + empty_grandparent_senders_and_authorities; + ankerl::unordered_dense::segmented_set
+ parent_senders_and_authorities; + parent_senders_and_authorities.insert(SENDER); // sender cannot dip + std::vector
const senders = {SENDER}; + std::vector>> const authorities = {{}}; + ankerl::unordered_dense::segmented_set
senders_and_authorities; + senders_and_authorities.insert(SENDER); + ChainContext const context{ + .grandparent_senders_and_authorities = + empty_grandparent_senders_and_authorities, + .parent_senders_and_authorities = parent_senders_and_authorities, + .senders_and_authorities = senders_and_authorities, + .senders = senders, + .authorities = authorities, + }; + + State state{bs, Incarnation{1, 1}}; + init_reserve_balance_context( + state, SENDER, tx, BASE_FEE_PER_GAS, 0, context); + state.subtract_from_balance(SENDER, sender_gas_fee); + + EXPECT_TRUE(revert_transaction( + SENDER, tx, BASE_FEE_PER_GAS, 0, state, context)); + EXPECT_TRUE(revert_transaction_cached(state)); + + uint256_t const sender_balance = state.get_balance(SENDER); + state.add_to_balance( + SENDER, std::numeric_limits::max() - sender_balance); + + EXPECT_TRUE(revert_transaction( + SENDER, tx, BASE_FEE_PER_GAS, 0, state, context)); + EXPECT_TRUE(revert_transaction_cached(state)); +} + TYPED_TEST(MonadTraitsTest, staking_contract_balance_drop_does_not_revert) { if constexpr (TestFixture::Trait::monad_rev() < MONAD_FOUR) { @@ -480,9 +557,9 @@ TYPED_TEST(MonadTraitsTest, can_sender_dip_into_reserve) TYPED_TEST(MonadTraitsTest, reserve_checks_code_hash) { using traits = typename TestFixture::Trait; - constexpr Address SENDER{1}; - constexpr Address NEW_CONTRACT{2}; - constexpr uint64_t BASE_FEE_PER_GAS = 10; + static constexpr Address SENDER{1}; + static constexpr Address NEW_CONTRACT{2}; + static constexpr uint64_t BASE_FEE_PER_GAS = 10; auto const to_wei = [](uint64_t mon) { return uint256_t{mon} * 1000000000000000000ULL; }; @@ -559,9 +636,9 @@ TYPED_TEST(MonadTraitsTest, reserve_checks_code_hash) TYPED_TEST(MonadTraitsTest, reserve_checks_empty_code_hash) { using traits = typename TestFixture::Trait; - constexpr Address SENDER{1}; - constexpr Address NEW_CONTRACT{2}; - constexpr uint64_t BASE_FEE_PER_GAS = 10; + static constexpr Address SENDER{1}; + static constexpr Address NEW_CONTRACT{2}; + static constexpr uint64_t BASE_FEE_PER_GAS = 10; auto const to_wei = [](uint64_t mon) { return uint256_t{mon} * 1000000000000000000ULL; }; diff --git a/category/execution/monad/reserve_balance.cpp b/category/execution/monad/reserve_balance.cpp index 11e8fc7c4..d51c7daa7 100644 --- a/category/execution/monad/reserve_balance.cpp +++ b/category/execution/monad/reserve_balance.cpp @@ -119,12 +119,6 @@ bool dipped_into_reserve( if (addr == sender) { if (!can_sender_dip_into_reserve( sender, i, effective_is_delegated, ctx)) { - // Safety: this assertion is recoverable because it can be - // triggered via RPC parameter setting. - MONAD_ASSERT_THROW( - violation_threshold.has_value(), - "gas fee greater than reserve for non-dipping " - "transaction"); return true; } // Skip if allowed to dip into reserve @@ -244,9 +238,21 @@ void ReserveBalance::update_violation_status(Address const &address) failed_.erase(address); return; } - MONAD_ASSERT_THROW( - sender_gas_fees_ <= reserve, - "gas fee greater than reserve for non-dipping transaction"); + if (sender_gas_fees_ > reserve) { + // This currently only happens in the RPC path. + // If we later use a more permissive reserve-balance design that + // accounts for credits to non-delegated accounts, this could + // also occur during speculative execution with stale pre-tx + // data. In that case, a retry is guaranteed, so what we do here + // will not matter in such cases. + // + // For RPC, treat this as a transaction revert: keep the + // threshold unset and the sender marked failed for this + // transaction. This avoids underflow in the subtraction below. + violation_threshold.reset(); + failed_.insert(address); + return; + } reserve = reserve - sender_gas_fees_; } violation_threshold = reserve; diff --git a/category/rpc/monad_executor_test.cpp b/category/rpc/monad_executor_test.cpp index 4dc17e819..28549cb4c 100644 --- a/category/rpc/monad_executor_test.cpp +++ b/category/rpc/monad_executor_test.cpp @@ -3694,8 +3694,7 @@ TEST_F(EthCallFixture, eth_call_reserve_balance_emptying) monad_state_override_destroy(state_override); } -// Check that gas < reserve assertion in reserve balance implementation doesn't -// crash the RPC process +// Check reserve-balance failure in eth_call returns explicit violation status. TEST_F(EthCallFixture, eth_call_reserve_balance_assertion) { for (uint64_t i = 0; i < 256; ++i) { @@ -3800,10 +3799,8 @@ TEST_F(EthCallFixture, eth_call_reserve_balance_assertion) true); f.get(); - EXPECT_EQ(ctx.result->status_code, EVMC_INTERNAL_ERROR); - EXPECT_EQ( - std::string_view{ctx.result->message}, - "gas fee greater than reserve for non-dipping transaction"); + EXPECT_EQ(ctx.result->status_code, EVMC_MONAD_RESERVE_BALANCE_VIOLATION); + EXPECT_EQ(ctx.result->message, nullptr); monad_executor_destroy(executor); monad_state_override_destroy(state_override);