test: add tests for verify_and_charge_gas_key_tx_ephemeral #14925
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14925 +/- ##
===========================================
+ Coverage 1.30% 68.85% +67.54%
===========================================
Files 724 918 +194
Lines 140386 200911 +60525
Branches 140386 200911 +60525
===========================================
+ Hits 1835 138328 +136493
+ Misses 138498 56643 -81855
- Partials 53 5940 +5887
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| #[test] | ||
| // TODO(gas-keys): Remove "nightly" once stable supports gas keys. | ||
| // TODO(spice-test): Assess if this test is relevant for spice and if yes fix it. |
There was a problem hiding this comment.
The reason it doesn't work in spice is the view queries are still relying on chunk extra. This is a broader problem to fix with all spice tests.
| } | ||
|
|
||
| #[test] | ||
| // TODO(gas-keys): Remove "nightly" once stable supports gas keys. |
There was a problem hiding this comment.
note: We need nightly because Chain::new() hardcoded PROTOCOL_VERSION like:
let max_num_shards =
runtime_adapter.get_shard_layout(PROTOCOL_VERSION).num_shards() as usize;
So if we set a version above the binary version we will run into trouble with the epoch manager.
4f98022 to
8ffc310
Compare
| let accounts: Vec<AccountId> = | ||
| (0..4).map(|i| format!("account{}", i).parse().unwrap()).collect(); | ||
|
|
||
| let shard_layout = ShardLayout::single_shard(); | ||
| let validators_spec = create_validators_spec(shard_layout.num_shards() as usize, 0); | ||
| let clients = validators_spec_clients_with_rpc(&validators_spec); | ||
| let initial_balance = Balance::from_near(1_000_000); | ||
| let genesis = TestLoopBuilder::new_genesis_builder() | ||
| .epoch_length(epoch_length) | ||
| .validators_spec(validators_spec) | ||
| .add_user_accounts_simple(&accounts, initial_balance) | ||
| .build(); | ||
| let epoch_config_store = TestEpochConfigBuilder::build_store_from_genesis(&genesis); | ||
|
|
||
| let mut env = TestLoopBuilder::new() | ||
| .genesis(genesis) | ||
| .epoch_config_store(epoch_config_store) | ||
| .clients(clients) | ||
| .track_all_shards() | ||
| .build() | ||
| .warmup(); |
There was a problem hiding this comment.
please take a look at the setup code in test-loop-tests/src/examples/multinode.rs, you can use more ergonomic helper functions here
There was a problem hiding this comment.
Thanks, I tried to make it like that file.
| rpc_node.submit_tx(add_key_tx); | ||
| env.test_loop.run_for(Duration::seconds(5)); | ||
| check_txs(&env.test_loop.data, &env.node_datas, &rpc_account_id(), &[add_key_hash]); |
| let clients = env | ||
| .node_datas | ||
| .iter() | ||
| .map(|data| &env.test_loop.data.get(&data.client_sender.actor_handle()).client) | ||
| .collect_vec(); | ||
| let response = clients.runtime_query( | ||
| account_id, | ||
| QueryRequest::ViewGasKeyNonces { | ||
| account_id: account_id.clone(), | ||
| public_key: public_key.clone(), | ||
| }, | ||
| ); | ||
| let QueryResponseKind::GasKeyNonces(nonces) = response.kind else { | ||
| panic!("Expected GasKeyNonces response"); | ||
| }; | ||
| nonces[nonce_index as usize] |
There was a problem hiding this comment.
use rpc_node.runtime_query instead, clients extension is kind of deprecated
8ffc310 to
d6824ad
Compare
Add support for gas key transactions, which require handling multiple nonces per key. I completely leave out the fund accounting and balance accounting changes from this PR and we can add them later to keep scope small. I added a TODO for this, and for now account's balance continues to pay all `tx_cost`. (This is a "safe" approach, gas keys are just like ordinary access keys with multiple nonces with this change). * Updated transaction verification logic to distinguish between regular and gas key transactions, calling `verify_and_charge_gas_key_tx_ephemeral` for the latter and updating nonce values accordingly. * Modified the runtime to prefetch gas key nonces in parallel with accounts and access keys, using a new `gas_key_nonces`. The cache in `prepare_transactions_extra` is also modified to accommodate gas key nonces. * Added a new `InvalidTxError::InvalidNonceIndex` variant to handle cases where a transaction specifies an invalid nonce index. I took an approach of having a separate code path for gas keys based on feedback from last attempt at this and further changes that will be introduced for balance handling. In case we want a _unified_ code path instead, an unpolished draft is available in 8bf4f28. Tests in separate PR to manage diff size: #14925 The separate code path introduces duplication, but we can mitigate that by refactoring common parts which is done here: #14923 (This PR is based on that branch) As usual I defer openapi & protocol schema changes to last step before merging.
850d36b to
3b2b212
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive test coverage for the GasKeys transaction and nonce handling functionality. The PR includes unit tests in runtime/runtime/src/verifier.rs, an integration test in runtime/runtime/src/tests/apply.rs, and a test loop test in test-loop-tests/src/tests/gas_keys.rs.
Changes:
- Added unit tests for
verify_and_charge_gas_key_tx_ephemeralcovering valid transactions, edge cases, and error conditions - Added integration test for applying gas key transactions through the full runtime
- Added test loop test for end-to-end gas key transaction flow
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test-loop-tests/src/tests/mod.rs | Added module declaration for gas_keys tests |
| test-loop-tests/src/tests/gas_keys.rs | New test loop test for gas key transactions including nonce updates and balance transfers |
| runtime/runtime/src/verifier.rs | Added unit tests for gas key transaction verification, including helper functions for test setup; also added code to handle gas key transactions in test helper |
| runtime/runtime/src/tests/apply.rs | Added integration test for applying gas key transactions through the runtime |
Comments suppressed due to low confidence (1)
runtime/runtime/src/verifier.rs:279
- Duplicate code block detected. Lines 273-279 are identical to lines 267-272, both checking if the access key is a gas key and returning the same error. One of these blocks should be removed.
let TransactionCost { gas_burnt, gas_remaining, receipt_gas_price, total_cost, burnt_amount } =
*transaction_cost;
let account_id = tx.signer_id();
let tx_nonce = tx.nonce().nonce();
verify_nonce(tx_nonce, access_key.nonce, block_height)?;
// Check and deduct balance
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add support for gas key transactions, which require handling multiple nonces per key. I completely leave out the fund accounting and balance accounting changes from this PR and we can add them later to keep scope small. I added a TODO for this, and for now account's balance continues to pay all `tx_cost`. (This is a "safe" approach, gas keys are just like ordinary access keys with multiple nonces with this change). * Updated transaction verification logic to distinguish between regular and gas key transactions, calling `verify_and_charge_gas_key_tx_ephemeral` for the latter and updating nonce values accordingly. * Modified the runtime to prefetch gas key nonces in parallel with accounts and access keys, using a new `gas_key_nonces`. The cache in `prepare_transactions_extra` is also modified to accommodate gas key nonces. * Added a new `InvalidTxError::InvalidNonceIndex` variant to handle cases where a transaction specifies an invalid nonce index. I took an approach of having a separate code path for gas keys based on feedback from last attempt at this and further changes that will be introduced for balance handling. In case we want a _unified_ code path instead, an unpolished draft is available in near@8bf4f28. Tests in separate PR to manage diff size: near#14925 The separate code path introduces duplication, but we can mitigate that by refactoring common parts which is done here: near#14923 (This PR is based on that branch) As usual I defer openapi & protocol schema changes to last step before merging.
Adds unit testing and a test loop test for GasKeys transaction/nonce handling. Initially I included an "integration test" in `chain/chain/src/runtime/tests.rs` but removed it in near@955a176 and added a basic test loop test instead. The reason is I don't want to expand `TestEnv` in any way and there is a bit of a broken relation between Chain & TestEnv when it comes to the validity period check. In any case I am happy to bring that test back or try some suggested workarounds or alternative testing strategies.
# Gas Keys ([NEP-611](near/NEPs#611)) Gas keys are a new type of access key that carries a pre-funded NEAR balance and multiple independent nonce sequences. When a transaction is signed with a gas key, gas costs are deducted from the gas key's balance rather than the signer's account balance (deposits like NEAR transfers still come from the account). ## Why do we need gas keys? - Enabling SPICE without a DoS vector. The SPICE project (Separation of Execution from Consensus) decouples transaction execution from block production: transactions are included in blocks first, and executed asynchronously later. This means chunk producers can no longer verify account balances at inclusion time. An attacker could drain an account's balance in one transaction, then flood the chain with many transactions signed by that account. All would be accepted (since execution hasn't caught up), consuming block space for free when they inevitably fail. Gas keys close this gap by requiring gas to be pre-committed in the key itself, so chunk producers can verify gas availability without access to the latest state. - Parallel transaction submission. Today, programmatic users who submit many transactions in parallel must manage multiple access keys (each with its own nonce sequence). A gas key supports up to 1,024 independent nonce slots via a single key, removing that complexity. # Context - NEP (if exists): near/NEPs#611 - Implementation: - near#14868 - near#14869 - near#14883 - near#14891 - near#14892 - near#14911 - near#14924 - near#14925 - near#14930 - near#14933 - near#14969 - near#14985 - near#15033 - near#15034 - near#15067 - near#15068 - near#15100 - near#15101 - near#15186 # Testing and QA - unit tests - test-loop: https://github.com/near/nearcore/blob/master/test-loop-tests/src/tests/gas_keys.rs - forknet (automated): near#15207 - forknet (manual testing, planned) # Checklist - [x] Link to nightly nayduck run: https://nayduck.nearone.org/#/run/4756 - [x] Update CHANGELOG.md to include this protocol feature in the `Unreleased` section.
Adds unit testing and a test loop test for GasKeys transaction/nonce handling.
Initially I included an "integration test" in
chain/chain/src/runtime/tests.rsbut removed it in 955a176 and added a basic test loop test instead.The reason is I don't want to expand
TestEnvin any way and there is a bit of a broken relation between Chain & TestEnv when it comes to the validity period check.In any case I am happy to bring that test back or try some suggested workarounds or alternative testing strategies.