feat(gas-keys): add NonceIndex and TransactionV1 modifications#14883
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #14883 +/- ##
==========================================
+ Coverage 68.70% 68.72% +0.02%
==========================================
Files 916 916
Lines 199686 199835 +149
Branches 199686 199835 +149
==========================================
+ Hits 137192 137335 +143
- Misses 56581 56587 +6
Partials 5913 5913
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:
|
darioush
changed the title
| [dev-dependencies] | ||
| near-crypto = { workspace = true, features = ["rand"] } | ||
| near-primitives = { workspace = true, features = ["test_utils"] } | ||
|
|
There was a problem hiding this comment.
Adding these allows running the pool tests separately like:
cargo test --package near-pool --features test_features --lib -- tests::test_order_nonce_with_nonce_index
darioush
force-pushed
the
darioush/gas-keys-transaction-v1
branch
from
e399c1a to
fcbbd11
Compare
darioush
force-pushed
the
darioush/gas-keys-transaction-v1
branch
from
fcbbd11 to
d2e11b2
Compare
|
If we think we may want the transaction/receipt priority in the future or that removing |
| @@ -1632,29 +1632,29 @@ pub struct SignedTransactionView { | |||
| pub nonce: Nonce, | |||
There was a problem hiding this comment.
Why don't we use TransactionNonce in SignedTransactionView and have nonce and nonce index separate?
There was a problem hiding this comment.
I did this as it seemed the most simple and doesn't change the type of any existing field.
If we change it to TransactionNonce, then in json we would have the simple nonces as "nonce": {"nonce": 200}, which I worry could break some downstream expectations.
We could also make it serde(untagged) to still have "nonce": 200 and then "nonce": {"nonce": 200, "nonce_index": 2} which seems fine if you prefer this.
| transactions.extend(generate_transactions_v1("alice.near", "alice.near", 1, 5, Some(1))); | ||
|
|
||
| let mut pool = TransactionPool::new(TEST_SEED, None, ""); | ||
| let mut rng = thread_rng(); |
There was a problem hiding this comment.
please avoid using thread_rng in tests, instead use deterministic one with hardcoded seed so that test failures are reproducible
| /// Simple nonce without index, used by ordinary access keys | ||
| Nonce { nonce: Nonce }, | ||
| /// Nonce with index, used by gas keys | ||
| NonceAndIndex { nonce: Nonce, nonce_index: NonceIndex }, |
There was a problem hiding this comment.
since this is gas-key specific maybe let's call it GasKeyNonce for now? we can always rename it later if needed
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
| if let Some(idx) = nonce_index { | ||
| v.extend_from_slice(&borsh::to_vec(&idx).unwrap()); | ||
| } |
There was a problem hiding this comment.
before this was unconditional and using None for access keys.
However this changed ordering, changing link_imbalance_ratio of ultra_slow_test_bandwidth_scheduler_three_shards_random_receipts resulting in nayduck failure
# 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.
3 participants
This PR aligns
TransactionV1with the specification of NEP-611. This includes:priority_feefrom the unusedTransactionV1(priority_feewill be retained in json output for backwards compatibility)noncetoTransactionNonce, which can include thenonce_index.I update the pool to use a key containing the
nonce_indexwhen relevant and add basic add/remove UT for it.I left removal of
ReceiptV1, updating transaction processing, and fixingtool/mirrorsfor separate PRs.Notably it is still not possible to use TransactionV1 which is gated by GasKeys protocol feature.