feat(gas-keys): charge gas on failed deposit for gas key txs by darioush · Pull Request #15031 · near/nearcore

darioush · 2026-02-09T15:43:27Z

When a gas key transaction fails because the account can't cover the deposit, gas is still charged from the gas key balance. This prevents SPICE spam where an attacker drains their account between submission and execution to get free transactions.
Refactored verifier functions (verify_and_charge_tx_ephemeral, verify_and_charge_gas_key_tx_ephemeral) to return a TxVerdict enum instead of mutating account/access key directly (see Avoid Input/Output arguments as part of verify_and_charge_tx_ephemeral #14965). Callers apply state changes via VerificationResult::apply().
Added NotEnoughBalanceForDeposit error variant.
test_gas_key_tx_deposit_insufficient_charges_gas (apply-level), test_gas_key_tx_deposit_failed_for_{account_balance, storage_stake} (verifier-level)
Will fix protocol schema and openapi.json after review.

codecov · 2026-02-09T16:02:39Z

Codecov Report

❌ Patch coverage is 82.81250% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.89%. Comparing base (32648f6) to head (bab8937).
⚠️ Report is 11 commits behind head on master.

Files with missing lines	Patch %	Lines
runtime/runtime/src/verifier.rs	83.43%	24 Missing and 2 partials ⚠️
core/primitives/src/errors.rs	0.00%	14 Missing ⚠️
chain/chain/src/runtime/mod.rs	84.21%	4 Missing and 2 partials ⚠️
.../runtime-params-estimator/src/estimator_context.rs	0.00%	5 Missing ⚠️
runtime/runtime/src/lib.rs	95.78%	1 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #15031      +/-   ##
==========================================
+ Coverage   68.85%   68.89%   +0.04%     
==========================================
  Files         920      920              
  Lines      201667   202330     +663     
  Branches   201667   202330     +663     
==========================================
+ Hits       138864   139405     +541     
- Misses      56823    56943     +120     
- Partials     5980     5982       +2

Flag	Coverage Δ
pytests-nightly	`1.29% <0.00%> (-0.01%)`	⬇️
unittests	`68.46% <80.31%> (-0.01%)`	⬇️
unittests-nightly	`68.46% <82.81%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR hardens gas key transactions by ensuring gas is still charged even when execution fails due to insufficient account deposit balance, preventing free “deposit-fail” spam. It also refactors verifier APIs to return a TxVerdict/VerificationResult describing state changes instead of mutating state directly.

Changes:

Charge gas key balance (and advance gas key nonce) on “deposit insufficient” failures via TxVerdict::DepositFailed.
Refactor verify_and_charge_tx_ephemeral / verify_and_charge_gas_key_tx_ephemeral to return TxVerdict, and apply state changes via VerificationResult::apply().
Add InvalidTxError::NotEnoughBalanceForDeposit and DepositCostFailureReason, plus tests covering the new behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test-loop-tests/src/tests/gas_keys.rs	Adds a TODO to cover deposit-insufficient gas key tx behavior in test-loop.
runtime/runtime/src/verifier.rs	Refactors verifier functions to return `TxVerdict`; adds gas-key deposit-failure handling and unit tests.
runtime/runtime/src/tests/apply.rs	Refactors test setup and adds an apply-level regression test ensuring gas is charged on deposit failure.
runtime/runtime/src/lib.rs	Introduces `TxVerdict`, `AccessKeyUpdate`, and `VerificationResult::apply()`; updates runtime processing to handle deposit-failed charging.
runtime/runtime-params-estimator/src/estimator_context.rs	Updates estimator to handle `TxVerdict` and apply state changes explicitly.
core/primitives/src/transaction.rs	Adds `ExecutionOutcomeWithId::failed_with_gas_burnt` to record gas/tokens burnt for specific failure cases.
core/primitives/src/errors.rs	Adds `DepositCostFailureReason` and `InvalidTxError::NotEnoughBalanceForDeposit` with display formatting.
chain/chain/src/runtime/mod.rs	Updates validation/filtering paths to handle `TxVerdict` and apply state updates on success.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-09T17:17:53Z

+            TxVerdict::Failed(e) | TxVerdict::DepositFailed { error: e, .. } => return Err(e),
        };
+        result.apply(&mut signer, &mut access_key);
        set_tx_state_changes(state_update, &validated_tx, &signer, &access_key);


validate_verify_and_charge_transaction applies VerificationResult to the in-memory signer/access_key and writes them back via set_tx_state_changes, but it never persists the gas key nonce update to the trie. For gas key transactions this means TrieKey::GasKeyNonce remains stale even on success. Persist result.gas_key_nonce_update() (when present) using set_gas_key_nonce(...) on the provided state_update.

Suggested change

set_tx_state_changes(state_update, &validated_tx, &signer, &access_key);

set_tx_state_changes(state_update, &validated_tx, &signer, &access_key);

// Persist gas key nonce updates (if any) to the trie so TrieKey::GasKeyNonce stays in sync.

if let Some(nonce_index) = tx.nonce().nonce_index() {

if let Some(new_nonce) = result.gas_key_nonce_update() {

set_gas_key_nonce(

state_update,

tx.signer_id(),

tx.public_key(),

nonce_index,

new_nonce,

);

}

}

This is not needed imo. Actually I would say it is a bug that some tests rely on set_tx_state_changes being called here. I don't think it's good to have a complex test helper here that's different from the production code.
We already have an apply level test to cover this case.

Copilot · 2026-02-09T17:17:53Z

+/// Returned by both `verify_and_charge_tx_ephemeral` and
+/// `verify_and_charge_gas_key_tx_ephemeral`. Neither function mutates state;
+/// callers apply changes based on the variant:
+/// - `Success`: apply all state changes via `VerificationResult::apply`.
+/// - `DepositFailed`: apply gas-only state changes via `VerificationResult::apply`
+///   (only returned by gas key path).
+/// - `Failed`: no state changes.


The TxVerdict doc comment says neither verify_and_charge_tx_ephemeral nor verify_and_charge_gas_key_tx_ephemeral mutates state, but verify_and_charge_tx_ephemeral can still mutate the access key allowance for legacy protocol versions (pre-FixAccessKeyAllowanceCharging). Please update this doc comment to reflect the legacy mutation so callers aren’t misled.

I prefer to leave the comment since the Legacy behavior will be removed soon.

darioush · 2026-02-09T15:48:50Z

        let transactions_gas_limit = chunk_tx_gas_limit(runtime_config, &prev_block, shard_id);

-        let mut result = PreparedTransactions::new();
+        let mut prepared_transactions = PreparedTransactions::new();


renamed to re-use "result" name in a more local scope

darioush · 2026-02-09T17:12:16Z

+///   (only returned by gas key path).
+/// - `Failed`: no state changes.
+#[derive(Debug)]
+pub enum TxVerdict {


I added this new enum (TxVerdict), one alternative that comes to mind is to rename it to VerificationResult and then rename VerificationResult to TxChanges or similar name.

I didn't include it to keep diff smaller, but can apply it if we think it's a good idea.

darioush · 2026-02-09T17:24:00Z

+            TxVerdict::Failed(e) | TxVerdict::DepositFailed { error: e, .. } => return Err(e),
        };
+        result.apply(&mut signer, &mut access_key);
        set_tx_state_changes(state_update, &validated_tx, &signer, &access_key);


This is not needed imo. Actually I would say it is a bug that some tests rely on set_tx_state_changes being called here. I don't think it's good to have a complex test helper here that's different from the production code.
We already have an apply level test to cover this case.

darioush · 2026-02-09T17:24:39Z

+/// Returned by both `verify_and_charge_tx_ephemeral` and
+/// `verify_and_charge_gas_key_tx_ephemeral`. Neither function mutates state;
+/// callers apply changes based on the variant:
+/// - `Success`: apply all state changes via `VerificationResult::apply`.
+/// - `DepositFailed`: apply gas-only state changes via `VerificationResult::apply`
+///   (only returned by gas key path).
+/// - `Failed`: no state changes.


I prefer to leave the comment since the Legacy behavior will be removed soon.

This is 1 out of 3 PRs for charging gas keys for failed transactions. See near/NEPs#611 for details. For reviewer's reference, all changes together: #15031 - Extracts shared test setup for gas key apply tests into a GasKeyTestSetup struct and setup_gas_key_test() function - Rewrites test_apply_gas_key_transaction and test_gas_refund_to_gas_key to use the shared helper - No behavior changes; purely a test refactor to reduce duplication ahead of adding more gas key tests

This is 2 out of 3 PRs for charging gas keys for failed transactions. See near/NEPs#611 for details. For reviewer's reference, all changes together: #15031 - Introduces `TxVerdict` enum (`Success`, `Failed`) replacing `Result<VerificationResult, InvalidTxError>` as the return type for transaction verification - Introduces `AccessKeyUpdate` enum (`Regular`, `GasKey`) to describe access key mutations declaratively Makes `verify_and_charge_tx_ephemeral` and `verify_and_charge_gas_key_tx_ephemeral` pure (except for Legacy bug): they no longer mutate `&mut Account` / `&mut AccessKey`; callers apply changes via `VerificationResult::apply()` - No behavior changes; this restructuring enables adding new verdict variants (e.g. `DepositFailed`) in follow-up work - Resolves #14965

This is 3 out of 3 PRs for charging gas keys for failed transactions. See near/NEPs#611 for details. For reviewer's reference, all changes together: #15031 - When a gas key transaction fails because the account can't cover the deposit, gas is still charged from the gas key balance. This prevents SPICE spam where an attacker drains their account between submission and execution to get free transactions. - Adds `TxVerdict::DepositFailed` variant: the verifier returns this when gas key balance is sufficient but the account can't cover the deposit - Adds `NotEnoughBalanceForDeposit` error variant - `test_gas_key_tx_deposit_insufficient_charges_gas` (apply-level), `test_gas_key_tx_deposit_failed_for_{account_balance, storage_stake}` (verifier-level) - Will fix protocol schema and openapi.json after review.

darioush · 2026-02-11T01:23:30Z

smaller PRs merged in instead

darioush added 4 commits February 9, 2026 07:42

feat(gas-keys): charge gas on failed deposit for gas key txs

26d0bdf

naming consistency

0912973

nit, ordering

0f83bab

nit, clarify comment

bab8937

darioush requested review from Copilot and removed request for Copilot February 9, 2026 16:38

Copilot started reviewing on behalf of darioush February 9, 2026 17:12 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

darioush added 2 commits February 9, 2026 09:25

fix string

10a221a

pass shard_uid through test setup

177b5d1

darioush commented Feb 9, 2026

View reviewed changes

This was referenced Feb 9, 2026

refactor(runtime): extract GasKeyTestSetup helper #15032

Merged

refactor(runtime): introduce TxVerdict, make verifiers pure #15033

Merged

feat(gas-keys): charge gas on failed deposit for gas key txs #15034

Merged

fix LackBalanceForState balance field, add test

f48eeea

darioush closed this Feb 11, 2026

darioush deleted the darioush/gaskeys-failedtx branch June 13, 2026 01:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(gas-keys): charge gas on failed deposit for gas key txs#15031

feat(gas-keys): charge gas on failed deposit for gas key txs#15031
darioush wants to merge 7 commits into
masterfrom
darioush/gaskeys-failedtx

darioush commented Feb 9, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

darioush Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

darioush Feb 9, 2026

Uh oh!

Uh oh!

Uh oh!

darioush Feb 9, 2026

Uh oh!

darioush Feb 9, 2026

Uh oh!

darioush Feb 9, 2026

Uh oh!

darioush Feb 9, 2026

Uh oh!

darioush commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        set_tx_state_changes(state_update, &validated_tx, &signer, &access_key);
+        set_tx_state_changes(state_update, &validated_tx, &signer, &access_key);
+        // Persist gas key nonce updates (if any) to the trie so TrieKey::GasKeyNonce stays in sync.
+        if let Some(nonce_index) = tx.nonce().nonce_index() {
+            if let Some(new_nonce) = result.gas_key_nonce_update() {
+                set_gas_key_nonce(
+                    state_update,
+                    tx.signer_id(),
+                    tx.public_key(),
+                    nonce_index,
+                    new_nonce,
+                );
+            }
+        }

Uh oh!

Conversation

darioush commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Feb 9, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

darioush Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

darioush Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

darioush Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

darioush Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

darioush Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

darioush Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

darioush commented Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

darioush commented Feb 9, 2026 •

edited

Loading