From ce474ebf849e5ff207b7b04e3dc54af1e058d283 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Tue, 30 Aug 2022 00:02:02 +0300 Subject: [PATCH 01/30] Draft implementation of DelegateAction (NEP-366) This is a draft implementation of NEP-366 (https://github.com/near/NEPs/pull/366) Not done yet: * Need to implement DelegateAction for implicit accounts (nonce problem) * Need new error codes for DelegateAction * Implement Fees for DelegateAction * Add tests --- chain/chain/src/test_utils.rs | 1 + chain/rosetta-rpc/src/adapters/mod.rs | 1 + core/primitives/src/receipt.rs | 30 ++++- core/primitives/src/transaction.rs | 40 ++++++ core/primitives/src/views.rs | 23 +++- .../genesis-csv-to-json/src/csv_parser.rs | 1 + .../src/tests/standard_cases/mod.rs | 1 + runtime/runtime/src/actions.rs | 50 +++++++- runtime/runtime/src/balance_checker.rs | 4 +- runtime/runtime/src/config.rs | 2 + runtime/runtime/src/lib.rs | 28 ++++- runtime/runtime/src/state_viewer/mod.rs | 1 + runtime/runtime/src/verifier.rs | 118 ++++++++++++++++++ 13 files changed, 291 insertions(+), 9 deletions(-) diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs index b29b54641d5..fc90a60d58f 100644 --- a/chain/chain/src/test_utils.rs +++ b/chain/chain/src/test_utils.rs @@ -861,6 +861,7 @@ impl RuntimeAdapter for KeyValueRuntime { receipt_id: create_receipt_nonce(from.clone(), to.clone(), amount, nonce), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: from.clone(), + publisher_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], diff --git a/chain/rosetta-rpc/src/adapters/mod.rs b/chain/rosetta-rpc/src/adapters/mod.rs index c3322cf308c..a3c56d5ac0c 100644 --- a/chain/rosetta-rpc/src/adapters/mod.rs +++ b/chain/rosetta-rpc/src/adapters/mod.rs @@ -410,6 +410,7 @@ impl From for Vec { ); operations.push(deploy_contract_operation); } + near_primitives::transaction::Action::Delegate(_) => todo!(), } } operations diff --git a/core/primitives/src/receipt.rs b/core/primitives/src/receipt.rs index 60e3345d507..c7ab867423b 100644 --- a/core/primitives/src/receipt.rs +++ b/core/primitives/src/receipt.rs @@ -10,7 +10,7 @@ use crate::borsh::maybestd::collections::HashMap; use crate::hash::CryptoHash; use crate::logging; use crate::serialize::{dec_format, option_base64_format}; -use crate::transaction::{Action, TransferAction}; +use crate::transaction::{Action, DelegateAction, TransferAction}; use crate::types::{AccountId, Balance, ShardId}; /// Receipts are used for a cross-shard communication. @@ -52,6 +52,7 @@ impl Receipt { receipt: ReceiptEnum::Action(ActionReceipt { signer_id: "system".parse().unwrap(), + publisher_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: 0, output_data_receivers: vec![], @@ -79,6 +80,7 @@ impl Receipt { receipt: ReceiptEnum::Action(ActionReceipt { signer_id: receiver_id.clone(), + publisher_id: None, signer_public_key, gas_price: 0, output_data_receivers: vec![], @@ -87,6 +89,30 @@ impl Receipt { }), } } + + pub fn new_delegate_actions( + publisher_id: &AccountId, + predecessor_id: &AccountId, + delegate_action: &DelegateAction, + public_key: &PublicKey, + gas_price: Balance, + ) -> Self { + Receipt { + predecessor_id: predecessor_id.clone(), + receiver_id: delegate_action.reciever_id.clone(), + receipt_id: CryptoHash::default(), + + receipt: ReceiptEnum::Action(ActionReceipt { + signer_id: predecessor_id.clone(), + publisher_id: Some(publisher_id.clone()), + signer_public_key: public_key.clone(), + gas_price: gas_price, + output_data_receivers: vec![], + input_data_ids: vec![], + actions: delegate_action.actions.clone(), + }), + } + } } /// Receipt could be either ActionReceipt or DataReceipt @@ -103,6 +129,8 @@ pub enum ReceiptEnum { pub struct ActionReceipt { /// A signer of the original transaction pub signer_id: AccountId, + /// A publisher's identifier in case of a delgated action + pub publisher_id: Option, /// An access key which was used to sign the original transaction pub signer_public_key: PublicKey, /// A gas_price which has been used to buy gas in the original transaction diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index c06cd8a1f2b..321dc05757b 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -1,6 +1,7 @@ use std::borrow::Borrow; use std::fmt; use std::hash::{Hash, Hasher}; +use std::io::Error; use borsh::{BorshDeserialize, BorshSerialize}; use serde::{Deserialize, Serialize}; @@ -70,6 +71,7 @@ pub enum Action { AddKey(AddKeyAction), DeleteKey(DeleteKeyAction), DeleteAccount(DeleteAccountAction), + Delegate(SignedDelegateAction), } impl Action { @@ -83,6 +85,10 @@ impl Action { match self { Action::FunctionCall(a) => a.deposit, Action::Transfer(a) => a.deposit, + Action::Delegate(a) => { + let delegate_action = a.get_delegate_action().unwrap(); + delegate_action.deposit + } _ => 0, } } @@ -220,6 +226,40 @@ impl From for Action { } } +#[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] +#[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] +pub struct DelegateAction { + pub reciever_id: AccountId, + pub deposit: Balance, + pub nonce: u64, + pub actions: Vec, +} +#[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] +#[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] +pub struct SignedDelegateAction { + // Borsh doesn't support recursive types. Therefore this field + // is deserialized to DelegateAction in runtime + pub delegate_action_serde: Vec, + pub public_key: PublicKey, + pub signature: Signature, +} + +impl SignedDelegateAction { + pub fn get_delegate_action(&self) -> Result { + DelegateAction::try_from_slice(&self.delegate_action_serde) + } + + pub fn get_hash(&self) -> CryptoHash { + hash(&self.delegate_action_serde) + } +} + +impl From for Action { + fn from(delegate_action: SignedDelegateAction) -> Self { + Self::Delegate(delegate_action) + } +} + #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] #[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, Eq, Debug, Clone)] #[borsh_init(init)] diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index 2594023b938..1ae0586d681 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -34,7 +34,8 @@ use crate::sharding::{ use crate::transaction::{ Action, AddKeyAction, CreateAccountAction, DeleteAccountAction, DeleteKeyAction, DeployContractAction, ExecutionMetadata, ExecutionOutcome, ExecutionOutcomeWithIdAndProof, - ExecutionStatus, FunctionCallAction, SignedTransaction, StakeAction, TransferAction, + ExecutionStatus, FunctionCallAction, SignedDelegateAction, SignedTransaction, StakeAction, + TransferAction, }; use crate::types::{ AccountId, AccountWithPublicKey, Balance, BlockHeight, CompiledContractCache, EpochHeight, @@ -954,6 +955,11 @@ pub enum ActionView { DeleteAccount { beneficiary_id: AccountId, }, + Delegate { + delegate_action_serde: Vec, + signature: Signature, + public_key: PublicKey, + }, } impl From for ActionView { @@ -982,6 +988,11 @@ impl From for ActionView { Action::DeleteAccount(action) => { ActionView::DeleteAccount { beneficiary_id: action.beneficiary_id } } + Action::Delegate(action) => ActionView::Delegate { + delegate_action_serde: action.delegate_action_serde, + signature: action.signature, + public_key: action.public_key, + }, } } } @@ -1011,6 +1022,15 @@ impl TryFrom for Action { ActionView::DeleteAccount { beneficiary_id } => { Action::DeleteAccount(DeleteAccountAction { beneficiary_id }) } + ActionView::Delegate { + delegate_action_serde: delegate_action, + signature, + public_key, + } => Action::Delegate(SignedDelegateAction { + delegate_action_serde: delegate_action, + signature, + public_key, + }), }) } } @@ -1478,6 +1498,7 @@ impl TryFrom for Receipt { actions, } => ReceiptEnum::Action(ActionReceipt { signer_id, + publisher_id: None, signer_public_key, gas_price, output_data_receivers: output_data_receivers diff --git a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs index f0e617be8bb..6af9f6b0570 100644 --- a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs +++ b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs @@ -255,6 +255,7 @@ fn account_records(row: &Row, gas_price: Balance) -> Vec { receipt_id: hash(row.account_id.as_ref().as_bytes()), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: row.account_id.clone(), + publisher_id: None, // `signer_public_key` can be anything because the key checks are not applied when // a transaction is already converted to a receipt. signer_public_key: PublicKey::empty(KeyType::ED25519), diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index 6b0bf330f77..c7ac7cd2b30 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -1414,6 +1414,7 @@ fn make_write_key_value_action(key: Vec, value: Vec) -> Action { fn make_receipt(node: &impl Node, actions: Vec, receiver_id: AccountId) -> Receipt { let receipt_enum = ReceiptEnum::Action(ActionReceipt { signer_id: alice_account(), + publisher_id: None, signer_public_key: node.signer().as_ref().public_key(), gas_price: 0, output_data_receivers: vec![], diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index d6bf5fb7aba..dfa58041422 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -11,7 +11,7 @@ use near_primitives::runtime::config::AccountCreationConfig; use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::transaction::{ Action, AddKeyAction, DeleteAccountAction, DeleteKeyAction, DeployContractAction, - FunctionCallAction, StakeAction, TransferAction, + FunctionCallAction, SignedDelegateAction, StakeAction, TransferAction, }; use near_primitives::types::validator_stake::ValidatorStake; use near_primitives::types::{AccountId, BlockHeight, EpochInfoProvider, TrieCacheMode}; @@ -30,6 +30,7 @@ use near_vm_errors::{ use near_vm_logic::types::PromiseResult; use near_vm_logic::VMContext; +use crate::balance_checker::receipt_cost; use crate::config::{safe_add_gas, RuntimeConfig}; use crate::ext::{ExternalError, RuntimeExt}; use crate::{ActionResult, ApplyState}; @@ -253,6 +254,7 @@ pub(crate) fn action_function_call( receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: action_receipt.signer_id.clone(), + publisher_id: action_receipt.publisher_id.clone(), signer_public_key: action_receipt.signer_public_key.clone(), gas_price: action_receipt.gas_price, output_data_receivers: receipt.output_data_receivers, @@ -613,6 +615,46 @@ pub(crate) fn action_add_key( Ok(()) } +pub(crate) fn action_delegate_action( + apply_state: &ApplyState, + action_receipt: &ActionReceipt, + predecessor_id: &AccountId, + signed_delegate_action: &SignedDelegateAction, + result: &mut ActionResult, +) -> Result<(), RuntimeError> { + match signed_delegate_action.get_delegate_action() { + Ok(delegate_action) => { + let new_receipt = Receipt::new_delegate_actions( + &action_receipt.signer_id, + predecessor_id, + &delegate_action, + &signed_delegate_action.public_key, + action_receipt.gas_price, + ); + + let transaction_costs = &apply_state.config.transaction_costs; + let current_protocol_version = apply_state.current_protocol_version; + let cost = receipt_cost(transaction_costs, current_protocol_version, &new_receipt)?; + + if let Some(refund) = delegate_action.deposit.checked_sub(cost.clone()) { + let refund_receipt = Receipt::new_balance_refund(&action_receipt.signer_id, refund); + + result.new_receipts.push(new_receipt); + result.new_receipts.push(refund_receipt); + } else { + result.result = Err(ActionErrorKind::LackBalanceForState { + account_id: action_receipt.signer_id.clone(), + amount: cost.clone(), + } + .into()); + } + } + Err(_) => todo!(), + } + + Ok(()) +} + pub(crate) fn check_actor_permissions( action: &Action, account: &Option, @@ -645,7 +687,10 @@ pub(crate) fn check_actor_permissions( .into()); } } - Action::CreateAccount(_) | Action::FunctionCall(_) | Action::Transfer(_) => (), + Action::CreateAccount(_) + | Action::FunctionCall(_) + | Action::Transfer(_) + | Action::Delegate(_) => (), }; Ok(()) } @@ -720,6 +765,7 @@ pub(crate) fn check_account_existence( .into()); } } + Action::Delegate(_) => (), }; Ok(()) } diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index 2a6b6c3ff92..bd1161d71e8 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -35,7 +35,7 @@ fn get_delayed_receipts( } /// Calculates and returns cost of a receipt. -fn receipt_cost( +pub(crate) fn receipt_cost( transaction_costs: &RuntimeFeesConfig, current_protocol_version: ProtocolVersion, receipt: &Receipt, @@ -422,6 +422,7 @@ mod tests { receipt_id: Default::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: tx.transaction.signer_id.clone(), + publisher_id: None, signer_public_key: tx.transaction.public_key.clone(), gas_price, output_data_receivers: vec![], @@ -477,6 +478,7 @@ mod tests { receipt_id: Default::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: tx.transaction.signer_id.clone(), + publisher_id: None, signer_public_key: tx.transaction.public_key.clone(), gas_price, output_data_receivers: vec![], diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index ee133a8788b..9ce34e02a04 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -120,6 +120,7 @@ pub fn total_send_fees( }, DeleteKey(_) => cfg.delete_key_cost.send_fee(sender_is_receiver), DeleteAccount(_) => cfg.delete_account_cost.send_fee(sender_is_receiver), + Delegate(_) => 0, // TODO: Set some fee }; result = safe_add_gas(result, delta)?; } @@ -170,6 +171,7 @@ pub fn exec_fee( }, DeleteKey(_) => cfg.delete_key_cost.exec_fee(), DeleteAccount(_) => cfg.delete_account_cost.exec_fee(), + Delegate(_) => cfg.delete_account_cost.exec_fee(), // TODO: Add another fee for Delegate action. } } diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 93b3302e9c7..fdfd575d6bd 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -251,6 +251,7 @@ impl Runtime { receipt_id, receipt: ReceiptEnum::Action(ActionReceipt { signer_id: transaction.signer_id.clone(), + publisher_id: None, signer_public_key: transaction.public_key.clone(), gas_price: verification_result.receipt_gas_price, output_data_receivers: vec![], @@ -442,6 +443,15 @@ impl Runtime { apply_state.current_protocol_version, )?; } + Action::Delegate(signed_delegate_action) => { + action_delegate_action( + apply_state, + action_receipt, + account_id, + signed_delegate_action, + &mut result, + )?; + } }; Ok(result) } @@ -792,16 +802,22 @@ impl Runtime { )?, )?; } + + let signer_is_predecessor = + action_receipt.signer_id.as_str() == receipt.predecessor_id.as_str(); + let refund_id = if action_receipt.publisher_id.is_some() && signer_is_predecessor { + action_receipt.publisher_id.as_ref().unwrap() + } else { + &receipt.predecessor_id + }; if deposit_refund > 0 { - result - .new_receipts - .push(Receipt::new_balance_refund(&receipt.predecessor_id, deposit_refund)); + result.new_receipts.push(Receipt::new_balance_refund(refund_id, deposit_refund)); } if gas_balance_refund > 0 { // Gas refunds refund the allowance of the access key, so if the key exists on the // account it will increase the allowance by the refund amount. result.new_receipts.push(Receipt::new_gas_refund( - &action_receipt.signer_id, + refund_id, gas_balance_refund, action_receipt.signer_public_key.clone(), )); @@ -1505,6 +1521,7 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: account_id, + publisher_id: None, signer_public_key: signer.public_key(), gas_price: GAS_PRICE, output_data_receivers: vec![], @@ -1855,6 +1872,7 @@ mod tests { receipt_id, receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), + publisher_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: GAS_PRICE, output_data_receivers: vec![], @@ -2170,6 +2188,7 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), + publisher_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], @@ -2239,6 +2258,7 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), + publisher_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index 1f72d098bb1..c1b48ec2d1c 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -212,6 +212,7 @@ impl TrieViewer { }; let action_receipt = ActionReceipt { signer_id: originator_id.clone(), + publisher_id: None, signer_public_key: public_key, gas_price: 0, output_data_receivers: vec![], diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index 3c43505bbc3..59eecf98308 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -1,5 +1,7 @@ use near_crypto::key_conversion::is_valid_staking_key; +use near_crypto::PublicKey; use near_primitives::runtime::get_insufficient_storage_stake; +use near_primitives::transaction::Transaction; use near_primitives::{ account::AccessKeyPermission, config::VMLimitConfig, @@ -92,6 +94,14 @@ pub fn verify_and_charge_transaction( let transaction = &signed_transaction.transaction; let signer_id = &transaction.signer_id; + verify_delegate_action( + state_update, + &config.wasm_config.limit_config, + transaction, + block_height, + current_protocol_version, + )?; + let mut signer = match get_account(state_update, signer_id)? { Some(signer) => signer, None => { @@ -329,7 +339,114 @@ pub fn validate_action( Action::AddKey(a) => validate_add_key_action(limit_config, a), Action::DeleteKey(_) => Ok(()), Action::DeleteAccount(_) => Ok(()), + Action::Delegate(_) => Ok(()), + } +} + +fn verify_delegate_action( + state_update: &mut TrieUpdate, + limit_config: &VMLimitConfig, + transaction: &Transaction, + #[allow(unused)] block_height: Option, + current_protocol_version: ProtocolVersion, +) -> Result<(), RuntimeError> { + let mut iter = transaction.actions.iter().peekable(); + let mut found_delegate_action = false; + while let Some(action) = iter.next() { + if let Action::Delegate(signed_delegate_action) = action { + if !found_delegate_action { + // There should be only one DelegateAction + found_delegate_action = true; + + let hash = signed_delegate_action.get_hash(); + let public_key = &signed_delegate_action.public_key; + if !signed_delegate_action.signature.verify(hash.as_ref(), public_key) { + return Err(InvalidTxError::InvalidSignature) + .map_err(RuntimeError::InvalidTxError); + } + + let delegate_action = signed_delegate_action.get_delegate_action().unwrap(); + validate_actions(limit_config, &delegate_action.actions) + .map_err(InvalidTxError::ActionsValidation)?; + + // DelegateAction shouldn't contain a nested DelegateAction + if delegate_action.actions.iter().any(|a| matches!(a, Action::Delegate(_))) { + return Err(ActionsValidationError::TotalNumberOfActionsExceeded { + total_number_of_actions: transaction.actions.len() as u64, + limit: 0, + }) + .map_err(InvalidTxError::ActionsValidation) + .map_err(RuntimeError::InvalidTxError); + } + + validate_access_key( + state_update, + &transaction.receiver_id, + &signed_delegate_action.public_key, + delegate_action.nonce, + block_height, + current_protocol_version, + )?; + } else { + return Err(ActionsValidationError::TotalNumberOfActionsExceeded { + total_number_of_actions: transaction.actions.len() as u64, + limit: 1, + }) + .map_err(InvalidTxError::ActionsValidation) + .map_err(RuntimeError::InvalidTxError); + } + } } + + Ok(()) +} + +pub fn validate_access_key( + state_update: &mut TrieUpdate, + signer_id: &AccountId, + public_key: &PublicKey, + nonce: u64, + #[allow(unused)] block_height: Option, + current_protocol_version: ProtocolVersion, +) -> Result<(), RuntimeError> { + match get_account(state_update, signer_id)? { + Some(signer) => signer, + None => { + return Err(InvalidTxError::SignerDoesNotExist { signer_id: signer_id.clone() }.into()); + } + }; + let mut access_key = match get_access_key(state_update, signer_id, public_key)? { + Some(access_key) => access_key, + None => { + return Err(InvalidTxError::InvalidAccessKeyError( + InvalidAccessKeyError::AccessKeyNotFound { + account_id: signer_id.clone(), + public_key: public_key.clone(), + }, + ) + .into()); + } + }; + + if nonce <= access_key.nonce { + return Err( + InvalidTxError::InvalidNonce { tx_nonce: nonce, ak_nonce: access_key.nonce }.into() + ); + } + if checked_feature!("stable", AccessKeyNonceRange, current_protocol_version) { + if let Some(height) = block_height { + let upper_bound = + height * near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER; + if nonce >= upper_bound { + return Err(InvalidTxError::NonceTooLarge { tx_nonce: nonce, upper_bound }.into()); + } + } + }; + + access_key.nonce = nonce; + + set_access_key(state_update, signer_id.clone(), public_key.clone(), &access_key); + Ok(()) } /// Validates `DeployContractAction`. Checks that the given contract size doesn't exceed the limit. @@ -1202,6 +1319,7 @@ mod tests { &limit_config, &ActionReceipt { signer_id: alice_account(), + publisher_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: 100, output_data_receivers: vec![], From 3eb7f027ff98f26e32052ee669ca325127c005f8 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 1 Sep 2022 00:33:31 +0300 Subject: [PATCH 02/30] Rename 'publisher_id' to 'relayer_id' --- chain/chain/src/test_utils.rs | 2 +- core/primitives/src/receipt.rs | 12 ++++++------ core/primitives/src/views.rs | 2 +- .../genesis-csv-to-json/src/csv_parser.rs | 2 +- integration-tests/src/tests/standard_cases/mod.rs | 2 +- runtime/runtime/src/actions.rs | 2 +- runtime/runtime/src/balance_checker.rs | 4 ++-- runtime/runtime/src/lib.rs | 14 +++++++------- runtime/runtime/src/state_viewer/mod.rs | 2 +- runtime/runtime/src/verifier.rs | 2 +- 10 files changed, 22 insertions(+), 22 deletions(-) diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs index fc90a60d58f..e0ab212b26b 100644 --- a/chain/chain/src/test_utils.rs +++ b/chain/chain/src/test_utils.rs @@ -861,7 +861,7 @@ impl RuntimeAdapter for KeyValueRuntime { receipt_id: create_receipt_nonce(from.clone(), to.clone(), amount, nonce), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: from.clone(), - publisher_id: None, + relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], diff --git a/core/primitives/src/receipt.rs b/core/primitives/src/receipt.rs index c7ab867423b..100ecabf7a8 100644 --- a/core/primitives/src/receipt.rs +++ b/core/primitives/src/receipt.rs @@ -52,7 +52,7 @@ impl Receipt { receipt: ReceiptEnum::Action(ActionReceipt { signer_id: "system".parse().unwrap(), - publisher_id: None, + relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: 0, output_data_receivers: vec![], @@ -80,7 +80,7 @@ impl Receipt { receipt: ReceiptEnum::Action(ActionReceipt { signer_id: receiver_id.clone(), - publisher_id: None, + relayer_id: None, signer_public_key, gas_price: 0, output_data_receivers: vec![], @@ -91,7 +91,7 @@ impl Receipt { } pub fn new_delegate_actions( - publisher_id: &AccountId, + relayer_id: &AccountId, predecessor_id: &AccountId, delegate_action: &DelegateAction, public_key: &PublicKey, @@ -104,7 +104,7 @@ impl Receipt { receipt: ReceiptEnum::Action(ActionReceipt { signer_id: predecessor_id.clone(), - publisher_id: Some(publisher_id.clone()), + relayer_id: Some(relayer_id.clone()), signer_public_key: public_key.clone(), gas_price: gas_price, output_data_receivers: vec![], @@ -129,8 +129,8 @@ pub enum ReceiptEnum { pub struct ActionReceipt { /// A signer of the original transaction pub signer_id: AccountId, - /// A publisher's identifier in case of a delgated action - pub publisher_id: Option, + /// A relayer's identifier in case of a delgated action + pub relayer_id: Option, /// An access key which was used to sign the original transaction pub signer_public_key: PublicKey, /// A gas_price which has been used to buy gas in the original transaction diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index 1ae0586d681..eab95c4f474 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -1498,7 +1498,7 @@ impl TryFrom for Receipt { actions, } => ReceiptEnum::Action(ActionReceipt { signer_id, - publisher_id: None, + relayer_id: None, signer_public_key, gas_price, output_data_receivers: output_data_receivers diff --git a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs index 6af9f6b0570..ecd04711d23 100644 --- a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs +++ b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs @@ -255,7 +255,7 @@ fn account_records(row: &Row, gas_price: Balance) -> Vec { receipt_id: hash(row.account_id.as_ref().as_bytes()), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: row.account_id.clone(), - publisher_id: None, + relayer_id: None, // `signer_public_key` can be anything because the key checks are not applied when // a transaction is already converted to a receipt. signer_public_key: PublicKey::empty(KeyType::ED25519), diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index c7ac7cd2b30..f44ee5dd6b6 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -1414,7 +1414,7 @@ fn make_write_key_value_action(key: Vec, value: Vec) -> Action { fn make_receipt(node: &impl Node, actions: Vec, receiver_id: AccountId) -> Receipt { let receipt_enum = ReceiptEnum::Action(ActionReceipt { signer_id: alice_account(), - publisher_id: None, + relayer_id: None, signer_public_key: node.signer().as_ref().public_key(), gas_price: 0, output_data_receivers: vec![], diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index dfa58041422..ad7e98bb9ec 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -254,7 +254,7 @@ pub(crate) fn action_function_call( receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: action_receipt.signer_id.clone(), - publisher_id: action_receipt.publisher_id.clone(), + relayer_id: action_receipt.relayer_id.clone(), signer_public_key: action_receipt.signer_public_key.clone(), gas_price: action_receipt.gas_price, output_data_receivers: receipt.output_data_receivers, diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index bd1161d71e8..c6805918d58 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -422,7 +422,7 @@ mod tests { receipt_id: Default::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: tx.transaction.signer_id.clone(), - publisher_id: None, + relayer_id: None, signer_public_key: tx.transaction.public_key.clone(), gas_price, output_data_receivers: vec![], @@ -478,7 +478,7 @@ mod tests { receipt_id: Default::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: tx.transaction.signer_id.clone(), - publisher_id: None, + relayer_id: None, signer_public_key: tx.transaction.public_key.clone(), gas_price, output_data_receivers: vec![], diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index fdfd575d6bd..5b815886624 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -251,7 +251,7 @@ impl Runtime { receipt_id, receipt: ReceiptEnum::Action(ActionReceipt { signer_id: transaction.signer_id.clone(), - publisher_id: None, + relayer_id: None, signer_public_key: transaction.public_key.clone(), gas_price: verification_result.receipt_gas_price, output_data_receivers: vec![], @@ -805,8 +805,8 @@ impl Runtime { let signer_is_predecessor = action_receipt.signer_id.as_str() == receipt.predecessor_id.as_str(); - let refund_id = if action_receipt.publisher_id.is_some() && signer_is_predecessor { - action_receipt.publisher_id.as_ref().unwrap() + let refund_id = if action_receipt.relayer_id.is_some() && signer_is_predecessor { + action_receipt.relayer_id.as_ref().unwrap() } else { &receipt.predecessor_id }; @@ -1521,7 +1521,7 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: account_id, - publisher_id: None, + relayer_id: None, signer_public_key: signer.public_key(), gas_price: GAS_PRICE, output_data_receivers: vec![], @@ -1872,7 +1872,7 @@ mod tests { receipt_id, receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), - publisher_id: None, + relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: GAS_PRICE, output_data_receivers: vec![], @@ -2188,7 +2188,7 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), - publisher_id: None, + relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], @@ -2258,7 +2258,7 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), - publisher_id: None, + relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index c1b48ec2d1c..8a70cbd1967 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -212,7 +212,7 @@ impl TrieViewer { }; let action_receipt = ActionReceipt { signer_id: originator_id.clone(), - publisher_id: None, + relayer_id: None, signer_public_key: public_key, gas_price: 0, output_data_receivers: vec![], diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index 59eecf98308..5a57b3d5444 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -1319,7 +1319,7 @@ mod tests { &limit_config, &ActionReceipt { signer_id: alice_account(), - publisher_id: None, + relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: 100, output_data_receivers: vec![], From 433b1583a8a3213416af7af1f80fc90889c8d4d1 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 1 Sep 2022 00:33:31 +0300 Subject: [PATCH 03/30] Updated according to the discussion in NEP-366 * Removed published_id * Added sender_id to DelegateAction * All things are verified when action is processing except verification of DelegateAction count in the transaction and verification wheher DelegateAction contains the nested ones * Added block_hash, nonce, public_key fields to DelegateAction * Gas is refunded to Transaction signer, deposit is refund to Receipt predecessor (because publisher_id has been removed) Need to be done: 1. `block_hash` isn't verified because there is not access to Store from `runtime' 2. Need to apply recommendation to `action_array_serde`: https://near.zulipchat.com/#narrow/stream/295302-general/topic/recursive.20types.20in.20borsh-rs/near/305181116 3. Need to add unit tests --- chain/chain/src/test_utils.rs | 1 - core/primitives/src/errors.rs | 33 +++ core/primitives/src/receipt.rs | 18 +- core/primitives/src/transaction.rs | 57 +++-- core/primitives/src/views.rs | 30 +-- .../genesis-csv-to-json/src/csv_parser.rs | 1 - .../src/tests/standard_cases/mod.rs | 1 - runtime/runtime/src/actions.rs | 224 ++++++++++++++---- runtime/runtime/src/balance_checker.rs | 4 +- runtime/runtime/src/config.rs | 47 +++- runtime/runtime/src/lib.rs | 26 +- runtime/runtime/src/state_viewer/mod.rs | 1 - runtime/runtime/src/verifier.rs | 137 +++-------- 13 files changed, 354 insertions(+), 226 deletions(-) diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs index e0ab212b26b..b29b54641d5 100644 --- a/chain/chain/src/test_utils.rs +++ b/chain/chain/src/test_utils.rs @@ -861,7 +861,6 @@ impl RuntimeAdapter for KeyValueRuntime { receipt_id: create_receipt_nonce(from.clone(), to.clone(), amount, nonce), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: from.clone(), - relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 260bc56085a..b92a5ee6781 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -200,6 +200,10 @@ pub enum ActionsValidationError { UnsuitableStakingKey { public_key: PublicKey }, /// The attached amount of gas in a FunctionCall action has to be a positive number. FunctionCallZeroAttachedGas, + /// The actions in DelegateAction are searilized incorrectly + DelegateActionDeserializeError, + /// DelegateAction actions contain another DelegateAction + DelegateActionCantContainNestedOne, } /// Describes the error for validating a receipt. @@ -317,6 +321,14 @@ impl Display for ActionsValidationError { f, "The attached amount of gas in a FunctionCall action has to be a positive number", ), + ActionsValidationError::DelegateActionDeserializeError => write!( + f, + "Can't deserialize actions in DelegateAction" + ), + ActionsValidationError::DelegateActionCantContainNestedOne => write!( + f, + "DelegateAction must not contain another DelegateAction" + ), } } } @@ -441,6 +453,20 @@ pub enum ActionErrorKind { OnlyImplicitAccountCreationAllowed { account_id: AccountId }, /// Delete account whose state is large is temporarily banned. DeleteAccountWithLargeState { account_id: AccountId }, + /// Signature does not match the provided actions and given signer public key. + DelegateActionInvalidSignature, + /// Receiver of the transaction doesn't match Sender of the delegate action + DelegateActionSenderDoesNotMatchTxReceiver { sender_id: AccountId, receiver_id: AccountId }, + /// Sender account doesn't exist + DelegateActionSenderDoesNotExist { sender_id: AccountId }, + /// Delegate action has expired + DelegateActionExpired, + /// The given public key doesn't exist for Sender account + DelegateActionAccessKeyError(InvalidAccessKeyError), + /// DelegateAction nonce must be greater sender[public_key].nonce + DelegateActionInvalidNonce { delegate_nonce: Nonce, ak_nonce: Nonce }, + /// DelegateAction nonce is larger than the upper bound given by the block height + DelegateActionNonceTooLarge { delegate_nonce: Nonce, upper_bound: Nonce }, } impl From for ActionError { @@ -751,6 +777,13 @@ impl Display for ActionErrorKind { ActionErrorKind::InsufficientStake { account_id, stake, minimum_stake } => write!(f, "Account {} tries to stake {} but minimum required stake is {}", account_id, stake, minimum_stake), ActionErrorKind::OnlyImplicitAccountCreationAllowed { account_id } => write!(f, "CreateAccount action is called on hex-characters account of length 64 {}", account_id), ActionErrorKind::DeleteAccountWithLargeState { account_id } => write!(f, "The state of account {} is too large and therefore cannot be deleted", account_id), + ActionErrorKind::DelegateActionInvalidSignature => write!(f, "DelegateAction is not signed with the given public key"), + ActionErrorKind::DelegateActionSenderDoesNotMatchTxReceiver { sender_id, receiver_id } => write!(f, "Transaction reciever {} doesn't match DelegateAction sender {}", sender_id, receiver_id), + ActionErrorKind::DelegateActionSenderDoesNotExist { sender_id } => write!(f, "DelegateAction sender account {} doesn't exist", sender_id), + ActionErrorKind::DelegateActionExpired => write!(f, "DelegateAction has expired"), + ActionErrorKind::DelegateActionAccessKeyError(access_key_error) => Display::fmt(&access_key_error, f), + ActionErrorKind::DelegateActionInvalidNonce { delegate_nonce, ak_nonce } => write!(f, "DelegateAction nonce {} must be larger than nonce of the used access key {}", delegate_nonce, ak_nonce), + ActionErrorKind::DelegateActionNonceTooLarge { delegate_nonce, upper_bound } => write!(f, "DelegateAction nonce {} must be smaller than the access key nonce upper bound {}", delegate_nonce, upper_bound), } } } diff --git a/core/primitives/src/receipt.rs b/core/primitives/src/receipt.rs index 100ecabf7a8..8f2f01f32ce 100644 --- a/core/primitives/src/receipt.rs +++ b/core/primitives/src/receipt.rs @@ -10,7 +10,7 @@ use crate::borsh::maybestd::collections::HashMap; use crate::hash::CryptoHash; use crate::logging; use crate::serialize::{dec_format, option_base64_format}; -use crate::transaction::{Action, DelegateAction, TransferAction}; +use crate::transaction::{Action, TransferAction}; use crate::types::{AccountId, Balance, ShardId}; /// Receipts are used for a cross-shard communication. @@ -52,7 +52,6 @@ impl Receipt { receipt: ReceiptEnum::Action(ActionReceipt { signer_id: "system".parse().unwrap(), - relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: 0, output_data_receivers: vec![], @@ -80,7 +79,6 @@ impl Receipt { receipt: ReceiptEnum::Action(ActionReceipt { signer_id: receiver_id.clone(), - relayer_id: None, signer_public_key, gas_price: 0, output_data_receivers: vec![], @@ -91,25 +89,25 @@ impl Receipt { } pub fn new_delegate_actions( - relayer_id: &AccountId, + signer_id: &AccountId, predecessor_id: &AccountId, - delegate_action: &DelegateAction, + receiver_id: &AccountId, + actions: &Vec, public_key: &PublicKey, gas_price: Balance, ) -> Self { Receipt { predecessor_id: predecessor_id.clone(), - receiver_id: delegate_action.reciever_id.clone(), + receiver_id: receiver_id.clone(), receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { - signer_id: predecessor_id.clone(), - relayer_id: Some(relayer_id.clone()), + signer_id: signer_id.clone(), signer_public_key: public_key.clone(), gas_price: gas_price, output_data_receivers: vec![], input_data_ids: vec![], - actions: delegate_action.actions.clone(), + actions: actions.clone(), }), } } @@ -129,8 +127,6 @@ pub enum ReceiptEnum { pub struct ActionReceipt { /// A signer of the original transaction pub signer_id: AccountId, - /// A relayer's identifier in case of a delgated action - pub relayer_id: Option, /// An access key which was used to sign the original transaction pub signer_public_key: PublicKey, /// A gas_price which has been used to buy gas in the original transaction diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 321dc05757b..5152b7a6be3 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -85,10 +85,6 @@ impl Action { match self { Action::FunctionCall(a) => a.deposit, Action::Transfer(a) => a.deposit, - Action::Delegate(a) => { - let delegate_action = a.get_delegate_action().unwrap(); - delegate_action.deposit - } _ => 0, } } @@ -226,31 +222,46 @@ impl From for Action { } } +#[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] +#[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] +pub struct ActionArraySerde { + /// A list of actions to be applied + actions: Vec, +} + #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] #[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] pub struct DelegateAction { - pub reciever_id: AccountId, - pub deposit: Balance, - pub nonce: u64, - pub actions: Vec, + /// Signer of the delegated actions + pub sender_id: AccountId, + /// Receiver of the delegated actions. + pub receiver_id: AccountId, + /// List of actions to be executed. + // This is a workaround to avoid a type recursion. 'actions[Action]' is deserialized in runtime. + // This field contains serialized ActionArraySerde structure. + pub action_array_serde: Vec, + /// Nonce to ensure that the same delegate action is not sent twice by a relayer and should match for given account's `public_key`. + /// After this action is processed it will increment. + pub nonce: Nonce, + /// The hash of the block in the blockchain on top of which the given DelegateAction is valid + pub block_hash: CryptoHash, + /// Public key that is used to sign this delegated action. + pub public_key: PublicKey, } #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] #[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] pub struct SignedDelegateAction { - // Borsh doesn't support recursive types. Therefore this field - // is deserialized to DelegateAction in runtime - pub delegate_action_serde: Vec, - pub public_key: PublicKey, + pub delegate_action: DelegateAction, pub signature: Signature, } impl SignedDelegateAction { - pub fn get_delegate_action(&self) -> Result { - DelegateAction::try_from_slice(&self.delegate_action_serde) - } + pub fn verify(&self) -> bool { + let delegate_action = &self.delegate_action; + let hash = delegate_action.get_hash(); + let public_key = &delegate_action.public_key; - pub fn get_hash(&self) -> CryptoHash { - hash(&self.delegate_action_serde) + self.signature.verify(hash.as_ref(), public_key) } } @@ -260,6 +271,18 @@ impl From for Action { } } +impl DelegateAction { + pub fn get_actions(&self) -> Result, Error> { + let a = ActionArraySerde::try_from_slice(&self.action_array_serde)?; + Ok(a.actions) + } + + pub fn get_hash(&self) -> CryptoHash { + let bytes = self.try_to_vec().expect("Failed to deserialize"); + hash(&bytes) + } +} + #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] #[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, Eq, Debug, Clone)] #[borsh_init(init)] diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index eab95c4f474..e57b399a083 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -32,10 +32,10 @@ use crate::sharding::{ ShardChunkHeaderV3, }; use crate::transaction::{ - Action, AddKeyAction, CreateAccountAction, DeleteAccountAction, DeleteKeyAction, - DeployContractAction, ExecutionMetadata, ExecutionOutcome, ExecutionOutcomeWithIdAndProof, - ExecutionStatus, FunctionCallAction, SignedDelegateAction, SignedTransaction, StakeAction, - TransferAction, + Action, AddKeyAction, CreateAccountAction, DelegateAction, DeleteAccountAction, + DeleteKeyAction, DeployContractAction, ExecutionMetadata, ExecutionOutcome, + ExecutionOutcomeWithIdAndProof, ExecutionStatus, FunctionCallAction, SignedDelegateAction, + SignedTransaction, StakeAction, TransferAction, }; use crate::types::{ AccountId, AccountWithPublicKey, Balance, BlockHeight, CompiledContractCache, EpochHeight, @@ -956,9 +956,8 @@ pub enum ActionView { beneficiary_id: AccountId, }, Delegate { - delegate_action_serde: Vec, + delegate_action: DelegateAction, signature: Signature, - public_key: PublicKey, }, } @@ -989,9 +988,8 @@ impl From for ActionView { ActionView::DeleteAccount { beneficiary_id: action.beneficiary_id } } Action::Delegate(action) => ActionView::Delegate { - delegate_action_serde: action.delegate_action_serde, + delegate_action: action.delegate_action, signature: action.signature, - public_key: action.public_key, }, } } @@ -1022,15 +1020,12 @@ impl TryFrom for Action { ActionView::DeleteAccount { beneficiary_id } => { Action::DeleteAccount(DeleteAccountAction { beneficiary_id }) } - ActionView::Delegate { - delegate_action_serde: delegate_action, - signature, - public_key, - } => Action::Delegate(SignedDelegateAction { - delegate_action_serde: delegate_action, - signature, - public_key, - }), + ActionView::Delegate { delegate_action, signature } => { + Action::Delegate(SignedDelegateAction { + delegate_action: delegate_action, + signature, + }) + } }) } } @@ -1498,7 +1493,6 @@ impl TryFrom for Receipt { actions, } => ReceiptEnum::Action(ActionReceipt { signer_id, - relayer_id: None, signer_public_key, gas_price, output_data_receivers: output_data_receivers diff --git a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs index ecd04711d23..f0e617be8bb 100644 --- a/genesis-tools/genesis-csv-to-json/src/csv_parser.rs +++ b/genesis-tools/genesis-csv-to-json/src/csv_parser.rs @@ -255,7 +255,6 @@ fn account_records(row: &Row, gas_price: Balance) -> Vec { receipt_id: hash(row.account_id.as_ref().as_bytes()), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: row.account_id.clone(), - relayer_id: None, // `signer_public_key` can be anything because the key checks are not applied when // a transaction is already converted to a receipt. signer_public_key: PublicKey::empty(KeyType::ED25519), diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index f44ee5dd6b6..6b0bf330f77 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -1414,7 +1414,6 @@ fn make_write_key_value_action(key: Vec, value: Vec) -> Action { fn make_receipt(node: &impl Node, actions: Vec, receiver_id: AccountId) -> Receipt { let receipt_enum = ReceiptEnum::Action(ActionReceipt { signer_id: alice_account(), - relayer_id: None, signer_public_key: node.signer().as_ref().public_key(), gas_price: 0, output_data_receivers: vec![], diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index ad7e98bb9ec..35ea44ae5ee 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -4,17 +4,21 @@ use near_crypto::PublicKey; use near_primitives::account::{AccessKey, AccessKeyPermission, Account}; use near_primitives::checked_feature; use near_primitives::contract::ContractCode; -use near_primitives::errors::{ActionError, ActionErrorKind, ContractCallError, RuntimeError}; +use near_primitives::errors::{ + ActionError, ActionErrorKind, ContractCallError, InvalidAccessKeyError, RuntimeError, +}; use near_primitives::hash::CryptoHash; use near_primitives::receipt::{ActionReceipt, Receipt, ReceiptEnum}; use near_primitives::runtime::config::AccountCreationConfig; use near_primitives::runtime::fees::RuntimeFeesConfig; use near_primitives::transaction::{ - Action, AddKeyAction, DeleteAccountAction, DeleteKeyAction, DeployContractAction, - FunctionCallAction, SignedDelegateAction, StakeAction, TransferAction, + Action, AddKeyAction, DelegateAction, DeleteAccountAction, DeleteKeyAction, + DeployContractAction, FunctionCallAction, SignedDelegateAction, StakeAction, TransferAction, }; use near_primitives::types::validator_stake::ValidatorStake; -use near_primitives::types::{AccountId, BlockHeight, EpochInfoProvider, TrieCacheMode}; +use near_primitives::types::{ + AccountId, Balance, BlockHeight, EpochInfoProvider, Gas, TrieCacheMode, +}; use near_primitives::utils::create_random_seed; use near_primitives::version::{ is_implicit_account_creation_enabled, ProtocolFeature, ProtocolVersion, @@ -30,8 +34,7 @@ use near_vm_errors::{ use near_vm_logic::types::PromiseResult; use near_vm_logic::VMContext; -use crate::balance_checker::receipt_cost; -use crate::config::{safe_add_gas, RuntimeConfig}; +use crate::config::{safe_add_gas, total_prepaid_exec_fees, total_prepaid_gas, RuntimeConfig}; use crate::ext::{ExternalError, RuntimeExt}; use crate::{ActionResult, ApplyState}; use near_primitives::config::ViewConfig; @@ -254,7 +257,6 @@ pub(crate) fn action_function_call( receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: action_receipt.signer_id.clone(), - relayer_id: action_receipt.relayer_id.clone(), signer_public_key: action_receipt.signer_public_key.clone(), gas_price: action_receipt.gas_price, output_data_receivers: receipt.output_data_receivers, @@ -353,11 +355,8 @@ pub(crate) fn try_refund_allowance( Ok(()) } -pub(crate) fn action_transfer( - account: &mut Account, - transfer: &TransferAction, -) -> Result<(), StorageError> { - account.set_amount(account.amount().checked_add(transfer.deposit).ok_or_else(|| { +pub(crate) fn action_transfer(account: &mut Account, deposit: Balance) -> Result<(), StorageError> { + account.set_amount(account.amount().checked_add(deposit).ok_or_else(|| { StorageError::StorageInconsistentState("Account balance integer overflow".to_string()) })?); Ok(()) @@ -414,7 +413,7 @@ pub(crate) fn action_implicit_account_creation_transfer( account: &mut Option, actor_id: &mut AccountId, account_id: &AccountId, - transfer: &TransferAction, + deposit: Balance, block_height: BlockHeight, current_protocol_version: ProtocolVersion, ) { @@ -443,7 +442,7 @@ pub(crate) fn action_implicit_account_creation_transfer( .expect("we should be able to deserialize ED25519 public key"); *account = Some(Account::new( - transfer.deposit, + deposit, 0, CryptoHash::default(), fee_config.storage_usage_config.num_bytes_account @@ -615,42 +614,183 @@ pub(crate) fn action_add_key( Ok(()) } -pub(crate) fn action_delegate_action( +pub(crate) fn apply_delegate_action( + state_update: &mut TrieUpdate, apply_state: &ApplyState, action_receipt: &ActionReceipt, - predecessor_id: &AccountId, + sender: &mut Option, + sender_id: &AccountId, signed_delegate_action: &SignedDelegateAction, result: &mut ActionResult, ) -> Result<(), RuntimeError> { - match signed_delegate_action.get_delegate_action() { - Ok(delegate_action) => { - let new_receipt = Receipt::new_delegate_actions( - &action_receipt.signer_id, - predecessor_id, - &delegate_action, - &signed_delegate_action.public_key, - action_receipt.gas_price, - ); - - let transaction_costs = &apply_state.config.transaction_costs; - let current_protocol_version = apply_state.current_protocol_version; - let cost = receipt_cost(transaction_costs, current_protocol_version, &new_receipt)?; - - if let Some(refund) = delegate_action.deposit.checked_sub(cost.clone()) { - let refund_receipt = Receipt::new_balance_refund(&action_receipt.signer_id, refund); - - result.new_receipts.push(new_receipt); - result.new_receipts.push(refund_receipt); - } else { - result.result = Err(ActionErrorKind::LackBalanceForState { - account_id: action_receipt.signer_id.clone(), - amount: cost.clone(), - } + let delegate_action = &signed_delegate_action.delegate_action; + + if !signed_delegate_action.verify() { + result.result = Err(ActionErrorKind::DelegateActionInvalidSignature.into()); + return Ok(()); + } + if delegate_action.sender_id.as_str() != sender_id.as_str() { + result.result = Err(ActionErrorKind::DelegateActionSenderDoesNotMatchTxReceiver { + sender_id: delegate_action.sender_id.clone(), + receiver_id: sender_id.clone(), + } + .into()); + return Ok(()); + } + if sender.is_none() { + result.result = + Err(ActionErrorKind::DelegateActionSenderDoesNotExist { sender_id: sender_id.clone() } .into()); - } + return Ok(()); + }; + + validate_access_key(state_update, apply_state, sender_id, delegate_action, result)?; + if result.result.is_err() { + return Ok(()); + } + + let actions = delegate_action.get_actions().unwrap(); + + let new_receipt = Receipt::new_delegate_actions( + &action_receipt.signer_id, + sender_id, + &delegate_action.receiver_id, + &actions, + &delegate_action.public_key, + action_receipt.gas_price, + ); + + let required_gas = receipt_required_gas(apply_state, &new_receipt)?; + result.gas_used += required_gas; + result.new_receipts.push(new_receipt); + + Ok(()) +} + +fn receipt_required_gas(apply_state: &ApplyState, receipt: &Receipt) -> Result { + Ok(match &receipt.receipt { + ReceiptEnum::Action(action_receipt) => { + let mut required_gas = safe_add_gas( + total_prepaid_exec_fees( + &apply_state.config.transaction_costs, + &action_receipt.actions, + &receipt.receiver_id, + apply_state.current_protocol_version, + )?, + total_prepaid_gas(&action_receipt.actions)?, + )?; + required_gas = safe_add_gas( + required_gas, + apply_state.config.transaction_costs.action_receipt_creation_config.exec_fee(), + )?; + + required_gas + } + ReceiptEnum::Data(_) => 0, + }) +} + +pub fn validate_access_key( + state_update: &mut TrieUpdate, + apply_state: &ApplyState, + sender_id: &AccountId, + delegate_action: &DelegateAction, + result: &mut ActionResult, +) -> Result<(), RuntimeError> { + let mut access_key = match get_access_key(state_update, sender_id, &delegate_action.public_key)? + { + Some(access_key) => access_key, + None => { + result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::AccessKeyNotFound { + account_id: sender_id.clone(), + public_key: delegate_action.public_key.clone(), + }, + ) + .into()); + return Ok(()); + } + }; + + if delegate_action.nonce <= access_key.nonce { + result.result = Err(ActionErrorKind::DelegateActionInvalidNonce { + delegate_nonce: delegate_action.nonce, + ak_nonce: access_key.nonce, } - Err(_) => todo!(), + .into()); + return Ok(()); } + if checked_feature!("stable", AccessKeyNonceRange, apply_state.current_protocol_version) { + let upper_bound = apply_state.block_index + * near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER; + if delegate_action.nonce >= upper_bound { + result.result = Err(ActionErrorKind::DelegateActionNonceTooLarge { + delegate_nonce: delegate_action.nonce, + upper_bound, + } + .into()); + return Ok(()); + } + }; + + access_key.nonce = delegate_action.nonce; + + let actions = delegate_action.get_actions().unwrap(); + + if let AccessKeyPermission::FunctionCall(ref function_call_permission) = access_key.permission { + if actions.len() != 1 { + result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::RequiresFullAccess, + ) + .into()); + return Ok(()); + } + if let Some(Action::FunctionCall(ref function_call)) = actions.get(0) { + if function_call.deposit > 0 { + result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::DepositWithFunctionCall, + ) + .into()); + } + if delegate_action.receiver_id.as_ref() != function_call_permission.receiver_id { + result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::ReceiverMismatch { + tx_receiver: delegate_action.receiver_id.clone(), + ak_receiver: function_call_permission.receiver_id.clone(), + }, + ) + .into()); + return Ok(()); + } + if !function_call_permission.method_names.is_empty() + && function_call_permission + .method_names + .iter() + .all(|method_name| &function_call.method_name != method_name) + { + result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::MethodNameMismatch { + method_name: function_call.method_name.clone(), + }, + ) + .into()); + return Ok(()); + } + } else { + result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::RequiresFullAccess, + ) + .into()); + return Ok(()); + } + }; + + set_access_key( + state_update, + sender_id.clone(), + delegate_action.public_key.clone(), + &access_key, + ); Ok(()) } diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index c6805918d58..2a6b6c3ff92 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -35,7 +35,7 @@ fn get_delayed_receipts( } /// Calculates and returns cost of a receipt. -pub(crate) fn receipt_cost( +fn receipt_cost( transaction_costs: &RuntimeFeesConfig, current_protocol_version: ProtocolVersion, receipt: &Receipt, @@ -422,7 +422,6 @@ mod tests { receipt_id: Default::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: tx.transaction.signer_id.clone(), - relayer_id: None, signer_public_key: tx.transaction.public_key.clone(), gas_price, output_data_receivers: vec![], @@ -478,7 +477,6 @@ mod tests { receipt_id: Default::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: tx.transaction.signer_id.clone(), - relayer_id: None, signer_public_key: tx.transaction.public_key.clone(), gas_price, output_data_receivers: vec![], diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index 9ce34e02a04..73336727977 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -120,7 +120,7 @@ pub fn total_send_fees( }, DeleteKey(_) => cfg.delete_key_cost.send_fee(sender_is_receiver), DeleteAccount(_) => cfg.delete_account_cost.send_fee(sender_is_receiver), - Delegate(_) => 0, // TODO: Set some fee + Delegate(_) => 1, // TODO: Set some fee }; result = safe_add_gas(result, delta)?; } @@ -241,7 +241,28 @@ pub fn total_prepaid_exec_fees( ) -> Result { let mut result = 0; for action in actions { - let delta = exec_fee(config, action, receiver_id, current_protocol_version); + let mut delta; + if let Action::Delegate(signed_delegate_action) = action { + let actions = signed_delegate_action.delegate_action.get_actions().unwrap(); + delta = total_prepaid_exec_fees( + config, + &actions, + &signed_delegate_action.delegate_action.receiver_id, + current_protocol_version, + )?; + delta = safe_add_gas( + delta, + exec_fee( + config, + action, + &signed_delegate_action.delegate_action.receiver_id, + current_protocol_version, + ), + )?; + delta = safe_add_gas(delta, config.action_receipt_creation_config.exec_fee())?; + } else { + delta = exec_fee(config, action, receiver_id, current_protocol_version); + } result = safe_add_gas(result, delta)?; } Ok(result) @@ -250,14 +271,32 @@ pub fn total_prepaid_exec_fees( pub fn total_deposit(actions: &[Action]) -> Result { let mut total_balance: Balance = 0; for action in actions { - total_balance = safe_add_balance(total_balance, action.get_deposit_balance())?; + let action_balance; + if let Action::Delegate(signed_delegate_action) = action { + let actions = signed_delegate_action.delegate_action.get_actions().unwrap(); + action_balance = total_deposit(&actions)?; + } else { + action_balance = action.get_deposit_balance(); + } + total_balance = safe_add_balance(total_balance, action_balance)?; } Ok(total_balance) } /// Get the total sum of prepaid gas for given actions. pub fn total_prepaid_gas(actions: &[Action]) -> Result { - actions.iter().try_fold(0, |acc, action| safe_add_gas(acc, action.get_prepaid_gas())) + let mut total_gas: Gas = 0; + for action in actions { + let action_gas; + if let Action::Delegate(signed_delegate_action) = action { + let actions = signed_delegate_action.delegate_action.get_actions().unwrap(); + action_gas = total_prepaid_gas(&actions)?; + } else { + action_gas = action.get_prepaid_gas(); + } + total_gas = safe_add_gas(total_gas, action_gas)?; + } + Ok(total_gas) } #[cfg(test)] diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 5b815886624..398fd482d10 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -251,7 +251,6 @@ impl Runtime { receipt_id, receipt: ReceiptEnum::Action(ActionReceipt { signer_id: transaction.signer_id.clone(), - relayer_id: None, signer_public_key: transaction.public_key.clone(), gas_price: verification_result.receipt_gas_price, output_data_receivers: vec![], @@ -372,7 +371,7 @@ impl Runtime { } Action::Transfer(transfer) => { if let Some(account) = account.as_mut() { - action_transfer(account, transfer)?; + action_transfer(account, transfer.deposit)?; // Check if this is a gas refund, then try to refund the access key allowance. if is_refund && action_receipt.signer_id == receipt.receiver_id { try_refund_allowance( @@ -394,7 +393,7 @@ impl Runtime { account, actor_id, &receipt.receiver_id, - transfer, + transfer.deposit, apply_state.block_index, apply_state.current_protocol_version, ); @@ -444,9 +443,11 @@ impl Runtime { )?; } Action::Delegate(signed_delegate_action) => { - action_delegate_action( + apply_delegate_action( + state_update, apply_state, action_receipt, + account, account_id, signed_delegate_action, &mut result, @@ -803,21 +804,16 @@ impl Runtime { )?; } - let signer_is_predecessor = - action_receipt.signer_id.as_str() == receipt.predecessor_id.as_str(); - let refund_id = if action_receipt.relayer_id.is_some() && signer_is_predecessor { - action_receipt.relayer_id.as_ref().unwrap() - } else { - &receipt.predecessor_id - }; if deposit_refund > 0 { - result.new_receipts.push(Receipt::new_balance_refund(refund_id, deposit_refund)); + result + .new_receipts + .push(Receipt::new_balance_refund(&receipt.predecessor_id, deposit_refund)); } if gas_balance_refund > 0 { // Gas refunds refund the allowance of the access key, so if the key exists on the // account it will increase the allowance by the refund amount. result.new_receipts.push(Receipt::new_gas_refund( - refund_id, + &action_receipt.signer_id, gas_balance_refund, action_receipt.signer_public_key.clone(), )); @@ -1521,7 +1517,6 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: account_id, - relayer_id: None, signer_public_key: signer.public_key(), gas_price: GAS_PRICE, output_data_receivers: vec![], @@ -1872,7 +1867,6 @@ mod tests { receipt_id, receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), - relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: GAS_PRICE, output_data_receivers: vec![], @@ -2188,7 +2182,6 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), - relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], @@ -2258,7 +2251,6 @@ mod tests { receipt_id: CryptoHash::default(), receipt: ReceiptEnum::Action(ActionReceipt { signer_id: bob_account(), - relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price, output_data_receivers: vec![], diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index 8a70cbd1967..1f72d098bb1 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -212,7 +212,6 @@ impl TrieViewer { }; let action_receipt = ActionReceipt { signer_id: originator_id.clone(), - relayer_id: None, signer_public_key: public_key, gas_price: 0, output_data_receivers: vec![], diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index 5a57b3d5444..96952f87f83 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -1,7 +1,6 @@ use near_crypto::key_conversion::is_valid_staking_key; -use near_crypto::PublicKey; use near_primitives::runtime::get_insufficient_storage_stake; -use near_primitives::transaction::Transaction; +use near_primitives::transaction::SignedDelegateAction; use near_primitives::{ account::AccessKeyPermission, config::VMLimitConfig, @@ -94,14 +93,6 @@ pub fn verify_and_charge_transaction( let transaction = &signed_transaction.transaction; let signer_id = &transaction.signer_id; - verify_delegate_action( - state_update, - &config.wasm_config.limit_config, - transaction, - block_height, - current_protocol_version, - )?; - let mut signer = match get_account(state_update, signer_id)? { Some(signer) => signer, None => { @@ -303,12 +294,25 @@ pub(crate) fn validate_actions( }); } + let mut found_delegate_action = false; let mut iter = actions.iter().peekable(); while let Some(action) = iter.next() { if let Action::DeleteAccount(_) = action { if iter.peek().is_some() { return Err(ActionsValidationError::DeleteActionMustBeFinal); } + } else if let Action::Delegate(signed_delegate_action) = action { + if found_delegate_action { + return Err(ActionsValidationError::DelegateActionCantContainNestedOne); + } + found_delegate_action = true; + + let delegate_actions = signed_delegate_action.delegate_action.get_actions(); + if let Ok(delegate_actions) = &delegate_actions { + validate_actions(limit_config, &delegate_actions)?; + } else { + return Err(ActionsValidationError::DelegateActionDeserializeError); + } } validate_action(limit_config, action)?; } @@ -339,113 +343,27 @@ pub fn validate_action( Action::AddKey(a) => validate_add_key_action(limit_config, a), Action::DeleteKey(_) => Ok(()), Action::DeleteAccount(_) => Ok(()), - Action::Delegate(_) => Ok(()), + Action::Delegate(a) => validate_delegate_action(limit_config, a), } } -fn verify_delegate_action( - state_update: &mut TrieUpdate, +fn validate_delegate_action( limit_config: &VMLimitConfig, - transaction: &Transaction, - #[allow(unused)] block_height: Option, - current_protocol_version: ProtocolVersion, -) -> Result<(), RuntimeError> { - let mut iter = transaction.actions.iter().peekable(); - let mut found_delegate_action = false; - while let Some(action) = iter.next() { - if let Action::Delegate(signed_delegate_action) = action { - if !found_delegate_action { - // There should be only one DelegateAction - found_delegate_action = true; - - let hash = signed_delegate_action.get_hash(); - let public_key = &signed_delegate_action.public_key; - if !signed_delegate_action.signature.verify(hash.as_ref(), public_key) { - return Err(InvalidTxError::InvalidSignature) - .map_err(RuntimeError::InvalidTxError); - } - - let delegate_action = signed_delegate_action.get_delegate_action().unwrap(); - validate_actions(limit_config, &delegate_action.actions) - .map_err(InvalidTxError::ActionsValidation)?; - - // DelegateAction shouldn't contain a nested DelegateAction - if delegate_action.actions.iter().any(|a| matches!(a, Action::Delegate(_))) { - return Err(ActionsValidationError::TotalNumberOfActionsExceeded { - total_number_of_actions: transaction.actions.len() as u64, - limit: 0, - }) - .map_err(InvalidTxError::ActionsValidation) - .map_err(RuntimeError::InvalidTxError); - } - - validate_access_key( - state_update, - &transaction.receiver_id, - &signed_delegate_action.public_key, - delegate_action.nonce, - block_height, - current_protocol_version, - )?; - } else { - return Err(ActionsValidationError::TotalNumberOfActionsExceeded { - total_number_of_actions: transaction.actions.len() as u64, - limit: 1, - }) - .map_err(InvalidTxError::ActionsValidation) - .map_err(RuntimeError::InvalidTxError); - } - } - } - - Ok(()) -} - -pub fn validate_access_key( - state_update: &mut TrieUpdate, - signer_id: &AccountId, - public_key: &PublicKey, - nonce: u64, - #[allow(unused)] block_height: Option, - current_protocol_version: ProtocolVersion, -) -> Result<(), RuntimeError> { - match get_account(state_update, signer_id)? { - Some(signer) => signer, - None => { - return Err(InvalidTxError::SignerDoesNotExist { signer_id: signer_id.clone() }.into()); - } - }; - let mut access_key = match get_access_key(state_update, signer_id, public_key)? { - Some(access_key) => access_key, - None => { - return Err(InvalidTxError::InvalidAccessKeyError( - InvalidAccessKeyError::AccessKeyNotFound { - account_id: signer_id.clone(), - public_key: public_key.clone(), - }, - ) - .into()); - } - }; + signed_delegate_action: &SignedDelegateAction, +) -> Result<(), ActionsValidationError> { + let delegate_action = &signed_delegate_action.delegate_action; + let actions = delegate_action.get_actions().unwrap(); - if nonce <= access_key.nonce { - return Err( - InvalidTxError::InvalidNonce { tx_nonce: nonce, ak_nonce: access_key.nonce }.into() - ); + // DelegateAction shouldn't contain a nested DelegateAction + if actions.iter().any(|a| matches!(a, Action::Delegate(_))) { + return Err(ActionsValidationError::TotalNumberOfActionsExceeded { + total_number_of_actions: actions.len() as u64, + limit: 0, + }); } - if checked_feature!("stable", AccessKeyNonceRange, current_protocol_version) { - if let Some(height) = block_height { - let upper_bound = - height * near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER; - if nonce >= upper_bound { - return Err(InvalidTxError::NonceTooLarge { tx_nonce: nonce, upper_bound }.into()); - } - } - }; - access_key.nonce = nonce; + validate_actions(limit_config, &actions)?; - set_access_key(state_update, signer_id.clone(), public_key.clone(), &access_key); Ok(()) } @@ -1319,7 +1237,6 @@ mod tests { &limit_config, &ActionReceipt { signer_id: alice_account(), - relayer_id: None, signer_public_key: PublicKey::empty(KeyType::ED25519), gas_price: 100, output_data_receivers: vec![], From a7bcf89fb2688eac3f3d447edd98d9a9bf8ec4c3 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Wed, 26 Oct 2022 14:39:49 +0300 Subject: [PATCH 04/30] Add fees for DelegateAction. --- core/primitives-core/src/parameter.rs | 4 ++++ core/primitives-core/src/runtime/fees.rs | 11 ++++++++++- core/primitives/res/runtime_configs/parameters.txt | 3 +++ .../res/runtime_configs/parameters_testnet.txt | 3 +++ core/primitives/src/runtime/parameter_table.rs | 1 + runtime/runtime-params-estimator/src/cost.rs | 3 +++ .../src/costs_to_runtime_config.rs | 1 + runtime/runtime/src/config.rs | 2 +- .../tests/runtime_group_tools/random_config.rs | 1 + 9 files changed, 27 insertions(+), 2 deletions(-) diff --git a/core/primitives-core/src/parameter.rs b/core/primitives-core/src/parameter.rs index 3e8669d4ef9..ff0f5ebc3cb 100644 --- a/core/primitives-core/src/parameter.rs +++ b/core/primitives-core/src/parameter.rs @@ -79,6 +79,9 @@ pub enum Parameter { ActionDeleteKeySendSir, ActionDeleteKeySendNotSir, ActionDeleteKeyExecution, + ActionDelegateSendSir, + ActionDelegateSendNotSir, + ActionDelegateExecution, // Smart contract dynamic gas costs WasmRegularOpCost, @@ -203,6 +206,7 @@ pub enum FeeParameter { ActionAddFunctionCallKey, ActionAddFunctionCallKeyPerByte, ActionDeleteKey, + ActionDelegate, } impl Parameter { diff --git a/core/primitives-core/src/runtime/fees.rs b/core/primitives-core/src/runtime/fees.rs index f02b4fed20d..e95d991278b 100644 --- a/core/primitives-core/src/runtime/fees.rs +++ b/core/primitives-core/src/runtime/fees.rs @@ -113,6 +113,9 @@ pub struct ActionCreationConfig { /// Base cost of deleting an account. pub delete_account_cost: Fee, + + /// Base cost of a delegate action + pub delegate_cost: Fee, } /// Describes the cost of creating an access key. @@ -219,6 +222,11 @@ impl RuntimeFeesConfig { send_not_sir: 147489000000, execution: 147489000000, }, + delegate_cost: Fee { + send_sir: 2319861500000, + send_not_sir: 2319861500000, + execution: 2319861500000, + }, }, storage_usage_config: StorageUsageConfig { // See Account in core/primitives/src/account.rs for the data structure. @@ -253,7 +261,8 @@ impl RuntimeFeesConfig { function_call_cost_per_byte: free.clone(), }, delete_key_cost: free.clone(), - delete_account_cost: free, + delete_account_cost: free.clone(), + delegate_cost: free, }, storage_usage_config: StorageUsageConfig { num_bytes_account: 0, diff --git a/core/primitives/res/runtime_configs/parameters.txt b/core/primitives/res/runtime_configs/parameters.txt index 62e72921272..527552e2511 100644 --- a/core/primitives/res/runtime_configs/parameters.txt +++ b/core/primitives/res/runtime_configs/parameters.txt @@ -66,6 +66,9 @@ action_add_function_call_key_per_byte_execution: 1_925_331 action_delete_key_send_sir: 94_946_625_000 action_delete_key_send_not_sir: 94_946_625_000 action_delete_key_execution: 94_946_625_000 +action_delegate_send_sir: 2_319_861_500_000 +action_delegate_send_not_sir: 2_319_861_500_000 +action_delegate_execution: 2_319_861_500_000 # Smart contract dynamic gas costs wasm_regular_op_cost: 3_856_371 diff --git a/core/primitives/res/runtime_configs/parameters_testnet.txt b/core/primitives/res/runtime_configs/parameters_testnet.txt index dde330f73d4..cbfa2a19c3a 100644 --- a/core/primitives/res/runtime_configs/parameters_testnet.txt +++ b/core/primitives/res/runtime_configs/parameters_testnet.txt @@ -62,6 +62,9 @@ action_add_function_call_key_per_byte_execution: 1_925_331 action_delete_key_send_sir: 94_946_625_000 action_delete_key_send_not_sir: 94_946_625_000 action_delete_key_execution: 94_946_625_000 +action_delegate_send_sir: 2_319_861_500_000 +action_delegate_send_not_sir: 2_319_861_500_000 +action_delegate_execution: 2_319_861_500_000 # Smart contract dynamic gas costs wasm_regular_op_cost: 3_856_371 diff --git a/core/primitives/src/runtime/parameter_table.rs b/core/primitives/src/runtime/parameter_table.rs index fb8c3ed4d18..f9b05a2cec6 100644 --- a/core/primitives/src/runtime/parameter_table.rs +++ b/core/primitives/src/runtime/parameter_table.rs @@ -125,6 +125,7 @@ impl ParameterTable { }, "delete_key_cost": self.fee_json(FeeParameter::ActionDeleteKey), "delete_account_cost": self.fee_json(FeeParameter::ActionDeleteAccount), + "delegate_cost": self.fee_json(FeeParameter::ActionDelegate), }, "storage_usage_config": { "num_bytes_account": self.get(Parameter::StorageNumBytesAccount), diff --git a/runtime/runtime-params-estimator/src/cost.rs b/runtime/runtime-params-estimator/src/cost.rs index 8b68452db47..68881a3dc8f 100644 --- a/runtime/runtime-params-estimator/src/cost.rs +++ b/runtime/runtime-params-estimator/src/cost.rs @@ -154,6 +154,9 @@ pub enum Cost { /// Subtract the base cost of creating a sir-receipt. /// (TODO[jakmeier] Consider different account states. ActionDeleteAccount, + /// Estimates `action_creation_config.delegate_cost` which is charged + /// for `DelegateAction` actions. + ActionDelegate, /// Estimates `wasm_config.ext_costs.base` which is intended to be charged /// once on every host function call. However, this is currently diff --git a/runtime/runtime-params-estimator/src/costs_to_runtime_config.rs b/runtime/runtime-params-estimator/src/costs_to_runtime_config.rs index a4655421450..c28c540988b 100644 --- a/runtime/runtime-params-estimator/src/costs_to_runtime_config.rs +++ b/runtime/runtime-params-estimator/src/costs_to_runtime_config.rs @@ -78,6 +78,7 @@ fn runtime_fees_config(cost_table: &CostTable) -> anyhow::Result cfg.delete_key_cost.send_fee(sender_is_receiver), DeleteAccount(_) => cfg.delete_account_cost.send_fee(sender_is_receiver), - Delegate(_) => 1, // TODO: Set some fee + Delegate(_) => cfg.delegate_cost.send_fee(sender_is_receiver), }; result = safe_add_gas(result, delta)?; } diff --git a/runtime/runtime/tests/runtime_group_tools/random_config.rs b/runtime/runtime/tests/runtime_group_tools/random_config.rs index be0b3838e54..260975401bf 100644 --- a/runtime/runtime/tests/runtime_group_tools/random_config.rs +++ b/runtime/runtime/tests/runtime_group_tools/random_config.rs @@ -35,6 +35,7 @@ pub fn random_config() -> RuntimeConfig { }, delete_key_cost: random_fee(), delete_account_cost: random_fee(), + delegate_cost: random_fee(), }, storage_usage_config: StorageUsageConfig { num_bytes_account: rng.next_u64() % 10000, From 529ca2c107fb2343e2ba52f63a9472e7e71da146 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Tue, 1 Nov 2022 02:18:42 +0200 Subject: [PATCH 05/30] Remove redundant code. Add DelegateActionMustBeOnlyOne error --- core/primitives/src/errors.rs | 6 +++++ runtime/runtime/src/actions.rs | 40 ++++++++++++++++----------------- runtime/runtime/src/lib.rs | 4 ++-- runtime/runtime/src/verifier.rs | 34 ++++++++++------------------ 4 files changed, 40 insertions(+), 44 deletions(-) diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index b92a5ee6781..e0f77d44829 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -204,6 +204,8 @@ pub enum ActionsValidationError { DelegateActionDeserializeError, /// DelegateAction actions contain another DelegateAction DelegateActionCantContainNestedOne, + /// There should be the only one DelegateAction + DelegateActionMustBeOnlyOne, } /// Describes the error for validating a receipt. @@ -329,6 +331,10 @@ impl Display for ActionsValidationError { f, "DelegateAction must not contain another DelegateAction" ), + ActionsValidationError::DelegateActionMustBeOnlyOne => write!( + f, + "The actions can contain the ony one DelegateAction" + ) } } } diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 35ea44ae5ee..a605f732289 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -16,9 +16,7 @@ use near_primitives::transaction::{ DeployContractAction, FunctionCallAction, SignedDelegateAction, StakeAction, TransferAction, }; use near_primitives::types::validator_stake::ValidatorStake; -use near_primitives::types::{ - AccountId, Balance, BlockHeight, EpochInfoProvider, Gas, TrieCacheMode, -}; +use near_primitives::types::{AccountId, BlockHeight, EpochInfoProvider, Gas, TrieCacheMode}; use near_primitives::utils::create_random_seed; use near_primitives::version::{ is_implicit_account_creation_enabled, ProtocolFeature, ProtocolVersion, @@ -355,8 +353,11 @@ pub(crate) fn try_refund_allowance( Ok(()) } -pub(crate) fn action_transfer(account: &mut Account, deposit: Balance) -> Result<(), StorageError> { - account.set_amount(account.amount().checked_add(deposit).ok_or_else(|| { +pub(crate) fn action_transfer( + account: &mut Account, + transfer: &TransferAction, +) -> Result<(), StorageError> { + account.set_amount(account.amount().checked_add(transfer.deposit).ok_or_else(|| { StorageError::StorageInconsistentState("Account balance integer overflow".to_string()) })?); Ok(()) @@ -413,7 +414,7 @@ pub(crate) fn action_implicit_account_creation_transfer( account: &mut Option, actor_id: &mut AccountId, account_id: &AccountId, - deposit: Balance, + transfer: &TransferAction, block_height: BlockHeight, current_protocol_version: ProtocolVersion, ) { @@ -442,7 +443,7 @@ pub(crate) fn action_implicit_account_creation_transfer( .expect("we should be able to deserialize ED25519 public key"); *account = Some(Account::new( - deposit, + transfer.deposit, 0, CryptoHash::default(), fee_config.storage_usage_config.num_bytes_account @@ -644,7 +645,7 @@ pub(crate) fn apply_delegate_action( return Ok(()); }; - validate_access_key(state_update, apply_state, sender_id, delegate_action, result)?; + validate_delegate_action_key(state_update, apply_state, sender_id, delegate_action, result)?; if result.result.is_err() { return Ok(()); } @@ -690,7 +691,7 @@ fn receipt_required_gas(apply_state: &ApplyState, receipt: &Receipt) -> Result= upper_bound { - result.result = Err(ActionErrorKind::DelegateActionNonceTooLarge { - delegate_nonce: delegate_action.nonce, - upper_bound, - } - .into()); - return Ok(()); + + let upper_bound = apply_state.block_index + * near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER; + if delegate_action.nonce >= upper_bound { + result.result = Err(ActionErrorKind::DelegateActionNonceTooLarge { + delegate_nonce: delegate_action.nonce, + upper_bound, } - }; + .into()); + return Ok(()); + } access_key.nonce = delegate_action.nonce; diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 398fd482d10..c1d24ac68f2 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -371,7 +371,7 @@ impl Runtime { } Action::Transfer(transfer) => { if let Some(account) = account.as_mut() { - action_transfer(account, transfer.deposit)?; + action_transfer(account, transfer)?; // Check if this is a gas refund, then try to refund the access key allowance. if is_refund && action_receipt.signer_id == receipt.receiver_id { try_refund_allowance( @@ -393,7 +393,7 @@ impl Runtime { account, actor_id, &receipt.receiver_id, - transfer.deposit, + transfer, apply_state.block_index, apply_state.current_protocol_version, ); diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index 96952f87f83..b09706396ec 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -301,18 +301,11 @@ pub(crate) fn validate_actions( if iter.peek().is_some() { return Err(ActionsValidationError::DeleteActionMustBeFinal); } - } else if let Action::Delegate(signed_delegate_action) = action { + } else if let Action::Delegate(_) = action { if found_delegate_action { - return Err(ActionsValidationError::DelegateActionCantContainNestedOne); + return Err(ActionsValidationError::DelegateActionMustBeOnlyOne); } found_delegate_action = true; - - let delegate_actions = signed_delegate_action.delegate_action.get_actions(); - if let Ok(delegate_actions) = &delegate_actions { - validate_actions(limit_config, &delegate_actions)?; - } else { - return Err(ActionsValidationError::DelegateActionDeserializeError); - } } validate_action(limit_config, action)?; } @@ -351,20 +344,17 @@ fn validate_delegate_action( limit_config: &VMLimitConfig, signed_delegate_action: &SignedDelegateAction, ) -> Result<(), ActionsValidationError> { - let delegate_action = &signed_delegate_action.delegate_action; - let actions = delegate_action.get_actions().unwrap(); - - // DelegateAction shouldn't contain a nested DelegateAction - if actions.iter().any(|a| matches!(a, Action::Delegate(_))) { - return Err(ActionsValidationError::TotalNumberOfActionsExceeded { - total_number_of_actions: actions.len() as u64, - limit: 0, - }); + let delegate_actions = signed_delegate_action.delegate_action.get_actions(); + if let Ok(delegate_actions) = &delegate_actions { + // DelegateAction shouldn't contain a nested DelegateAction + if delegate_actions.iter().any(|a| matches!(a, Action::Delegate(_))) { + return Err(ActionsValidationError::DelegateActionCantContainNestedOne); + } + validate_actions(limit_config, &delegate_actions)?; + Ok(()) + } else { + Err(ActionsValidationError::DelegateActionDeserializeError) } - - validate_actions(limit_config, &actions)?; - - Ok(()) } /// Validates `DeployContractAction`. Checks that the given contract size doesn't exceed the limit. From cfb97806d1e3cce2d2bc7f64013ce4658f1ba637 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Wed, 2 Nov 2022 00:38:44 +0200 Subject: [PATCH 06/30] Add the inner actions' send fees to the total send fee --- runtime/runtime/src/config.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index 42cb634a86e..2ae6d778353 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -120,7 +120,20 @@ pub fn total_send_fees( }, DeleteKey(_) => cfg.delete_key_cost.send_fee(sender_is_receiver), DeleteAccount(_) => cfg.delete_account_cost.send_fee(sender_is_receiver), - Delegate(_) => cfg.delegate_cost.send_fee(sender_is_receiver), + Delegate(signed_delegate_action) => { + let delegate_cost = cfg.delegate_cost.send_fee(sender_is_receiver); + let delegate_action = &signed_delegate_action.delegate_action; + let sender_is_receiver = delegate_action.sender_id == delegate_action.receiver_id; + + delegate_cost + + total_send_fees( + config, + sender_is_receiver, + &delegate_action.get_actions().unwrap(), + &delegate_action.receiver_id, + current_protocol_version, + )? + } }; result = safe_add_gas(result, delta)?; } From 602baca01fb2b3cd8d562b15540238ebe76d9f9d Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Wed, 2 Nov 2022 23:32:02 +0200 Subject: [PATCH 07/30] Charge the send fee for the inner actions twice --- runtime/runtime/src/config.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index 2ae6d778353..6274a900bdd 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -125,8 +125,11 @@ pub fn total_send_fees( let delegate_action = &signed_delegate_action.delegate_action; let sender_is_receiver = delegate_action.sender_id == delegate_action.receiver_id; + // `total_send_fees` is multiplied 2 because the first fee is charged for sending + // the inner actions to the tx receiver shard, the second fee is charged for sending + // the inner actions from the tx receiver shard to the delegate action receiver shard delegate_cost - + total_send_fees( + + 2 * total_send_fees( config, sender_is_receiver, &delegate_action.get_actions().unwrap(), From 3b78721092891ce8367acda66b2f8c4e29e95ea9 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Tue, 1 Nov 2022 01:47:10 +0200 Subject: [PATCH 08/30] Refactor deserialization the inner actions * Handle a nested DelegateAction while the parent one is deserialized --- core/primitives/src/transaction.rs | 36 +++++++++++++++++++++--------- runtime/runtime/src/actions.rs | 4 ++-- runtime/runtime/src/config.rs | 8 +++---- runtime/runtime/src/verifier.rs | 14 +++--------- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 5152b7a6be3..6396c5aaf9d 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -1,7 +1,7 @@ use std::borrow::Borrow; use std::fmt; use std::hash::{Hash, Hasher}; -use std::io::Error; +use std::io::{Error, ErrorKind}; use borsh::{BorshDeserialize, BorshSerialize}; use serde::{Deserialize, Serialize}; @@ -223,10 +223,27 @@ impl From for Action { } #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] -#[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] -pub struct ActionArraySerde { - /// A list of actions to be applied - actions: Vec, +#[derive(Serialize, BorshSerialize, Deserialize, PartialEq, Eq, Clone, Debug)] +pub struct NonDelegateAction(Action); + +const ACTION_DELEGATE_NUMBER: u8 = 8; + +impl From<&NonDelegateAction> for Action { + fn from(action: &NonDelegateAction) -> Self { + action.0.clone() + } +} + +impl borsh::de::BorshDeserialize for NonDelegateAction { + fn deserialize(buf: &mut &[u8]) -> ::core::result::Result { + match buf[0] { + ACTION_DELEGATE_NUMBER => Err(Error::new( + ErrorKind::InvalidInput, + "DelegateAction mustn't contain a nested one", + )), + _ => Ok(Self(borsh::BorshDeserialize::deserialize(buf)?)), + } + } } #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] @@ -237,9 +254,7 @@ pub struct DelegateAction { /// Receiver of the delegated actions. pub receiver_id: AccountId, /// List of actions to be executed. - // This is a workaround to avoid a type recursion. 'actions[Action]' is deserialized in runtime. - // This field contains serialized ActionArraySerde structure. - pub action_array_serde: Vec, + pub actions: Vec, /// Nonce to ensure that the same delegate action is not sent twice by a relayer and should match for given account's `public_key`. /// After this action is processed it will increment. pub nonce: Nonce, @@ -272,9 +287,8 @@ impl From for Action { } impl DelegateAction { - pub fn get_actions(&self) -> Result, Error> { - let a = ActionArraySerde::try_from_slice(&self.action_array_serde)?; - Ok(a.actions) + pub fn get_actions(&self) -> Vec { + self.actions.iter().map(|a| a.into()).collect() } pub fn get_hash(&self) -> CryptoHash { diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index a605f732289..1b77fba58f7 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -650,7 +650,7 @@ pub(crate) fn apply_delegate_action( return Ok(()); } - let actions = delegate_action.get_actions().unwrap(); + let actions = delegate_action.get_actions(); let new_receipt = Receipt::new_delegate_actions( &action_receipt.signer_id, @@ -735,7 +735,7 @@ fn validate_delegate_action_key( access_key.nonce = delegate_action.nonce; - let actions = delegate_action.get_actions().unwrap(); + let actions = delegate_action.get_actions(); if let AccessKeyPermission::FunctionCall(ref function_call_permission) = access_key.permission { if actions.len() != 1 { diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index 6274a900bdd..1b95543d1e0 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -132,7 +132,7 @@ pub fn total_send_fees( + 2 * total_send_fees( config, sender_is_receiver, - &delegate_action.get_actions().unwrap(), + &delegate_action.get_actions(), &delegate_action.receiver_id, current_protocol_version, )? @@ -259,7 +259,7 @@ pub fn total_prepaid_exec_fees( for action in actions { let mut delta; if let Action::Delegate(signed_delegate_action) = action { - let actions = signed_delegate_action.delegate_action.get_actions().unwrap(); + let actions = signed_delegate_action.delegate_action.get_actions(); delta = total_prepaid_exec_fees( config, &actions, @@ -289,7 +289,7 @@ pub fn total_deposit(actions: &[Action]) -> Result Result Result<(), ActionsValidationError> { - let delegate_actions = signed_delegate_action.delegate_action.get_actions(); - if let Ok(delegate_actions) = &delegate_actions { - // DelegateAction shouldn't contain a nested DelegateAction - if delegate_actions.iter().any(|a| matches!(a, Action::Delegate(_))) { - return Err(ActionsValidationError::DelegateActionCantContainNestedOne); - } - validate_actions(limit_config, &delegate_actions)?; - Ok(()) - } else { - Err(ActionsValidationError::DelegateActionDeserializeError) - } + let actions = signed_delegate_action.delegate_action.get_actions(); + validate_actions(limit_config, &actions)?; + Ok(()) } /// Validates `DeployContractAction`. Checks that the given contract size doesn't exceed the limit. From 106dc67afb1e06324a9dd44f17522382fd37b06f Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Wed, 2 Nov 2022 00:22:34 +0200 Subject: [PATCH 09/30] Applied max_block_height --- core/primitives/src/errors.rs | 2 +- core/primitives/src/transaction.rs | 5 +++-- runtime/runtime/src/actions.rs | 4 ++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index e0f77d44829..8c7d9b0f2fd 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -465,7 +465,7 @@ pub enum ActionErrorKind { DelegateActionSenderDoesNotMatchTxReceiver { sender_id: AccountId, receiver_id: AccountId }, /// Sender account doesn't exist DelegateActionSenderDoesNotExist { sender_id: AccountId }, - /// Delegate action has expired + /// Delegate action has expired. `max_block_height` is less than actual block height. DelegateActionExpired, /// The given public key doesn't exist for Sender account DelegateActionAccessKeyError(InvalidAccessKeyError), diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 6396c5aaf9d..dd86fe7964c 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -4,6 +4,7 @@ use std::hash::{Hash, Hasher}; use std::io::{Error, ErrorKind}; use borsh::{BorshDeserialize, BorshSerialize}; +use near_primitives_core::types::BlockHeight; use serde::{Deserialize, Serialize}; use near_crypto::{PublicKey, Signature}; @@ -258,8 +259,8 @@ pub struct DelegateAction { /// Nonce to ensure that the same delegate action is not sent twice by a relayer and should match for given account's `public_key`. /// After this action is processed it will increment. pub nonce: Nonce, - /// The hash of the block in the blockchain on top of which the given DelegateAction is valid - pub block_hash: CryptoHash, + /// The maximal height of the block in the blockchain below which the given DelegateAction is valid. + pub max_block_height: BlockHeight, /// Public key that is used to sign this delegated action. pub public_key: PublicKey, } diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 1b77fba58f7..44f817ddbd1 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -630,6 +630,10 @@ pub(crate) fn apply_delegate_action( result.result = Err(ActionErrorKind::DelegateActionInvalidSignature.into()); return Ok(()); } + if apply_state.block_index > delegate_action.max_block_height { + result.result = Err(ActionErrorKind::DelegateActionExpired.into()); + return Ok(()); + } if delegate_action.sender_id.as_str() != sender_id.as_str() { result.result = Err(ActionErrorKind::DelegateActionSenderDoesNotMatchTxReceiver { sender_id: delegate_action.sender_id.clone(), From ee124a64f4c4c8ebffd0108cef1c89b67b8c097f Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Fri, 4 Nov 2022 01:26:31 +0200 Subject: [PATCH 10/30] Fix delegate_cost.exec_fee todo --- runtime/runtime/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index 1b95543d1e0..7688939115c 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -187,7 +187,7 @@ pub fn exec_fee( }, DeleteKey(_) => cfg.delete_key_cost.exec_fee(), DeleteAccount(_) => cfg.delete_account_cost.exec_fee(), - Delegate(_) => cfg.delete_account_cost.exec_fee(), // TODO: Add another fee for Delegate action. + Delegate(_) => cfg.delegate_cost.exec_fee(), } } From 11c29fccd2c1eca1d4fcc387a0bd7c82219c488c Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Tue, 8 Nov 2022 21:00:59 +0200 Subject: [PATCH 11/30] Charge DelegateAction's 'send_fee' in two steps 1. Charge the inner actions' `send_fee` on Relayer shard and prepaid the `send_fee` for Sender shard 2. Burn the inner actions' `send_fee` on Sender shard (when DelegateAction is processed) --- runtime/runtime/src/actions.rs | 12 +++++++- runtime/runtime/src/balance_checker.rs | 10 ++++++- runtime/runtime/src/config.rs | 40 ++++++++++++++++++++++---- runtime/runtime/src/lib.rs | 10 ++++++- 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 44f817ddbd1..0afb4e1f79f 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -32,7 +32,10 @@ use near_vm_errors::{ use near_vm_logic::types::PromiseResult; use near_vm_logic::VMContext; -use crate::config::{safe_add_gas, total_prepaid_exec_fees, total_prepaid_gas, RuntimeConfig}; +use crate::config::{ + safe_add_gas, total_prepaid_exec_fees, total_prepaid_gas, total_prepaid_send_fees, + RuntimeConfig, +}; use crate::ext::{ExternalError, RuntimeExt}; use crate::{ActionResult, ApplyState}; use near_primitives::config::ViewConfig; @@ -665,8 +668,15 @@ pub(crate) fn apply_delegate_action( action_receipt.gas_price, ); + let prepaid_send_fees = total_prepaid_send_fees( + &apply_state.config.transaction_costs, + &action_receipt.actions, + apply_state.current_protocol_version, + )?; let required_gas = receipt_required_gas(apply_state, &new_receipt)?; result.gas_used += required_gas; + result.gas_used += prepaid_send_fees; + result.gas_burnt += prepaid_send_fees; result.new_receipts.push(new_receipt); Ok(()) diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index 2a6b6c3ff92..557fb0bd316 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -2,7 +2,7 @@ use crate::safe_add_balance_apply; use crate::config::{ safe_add_balance, safe_add_gas, safe_gas_to_balance, total_deposit, total_prepaid_exec_fees, - total_prepaid_gas, + total_prepaid_gas, total_prepaid_send_fees, }; use crate::{ApplyStats, DelayedReceiptIndices, ValidatorAccountsUpdate}; use near_primitives::errors::{ @@ -54,6 +54,14 @@ fn receipt_cost( )?, )?; total_gas = safe_add_gas(total_gas, total_prepaid_gas(&action_receipt.actions)?)?; + total_gas = safe_add_gas( + total_gas, + total_prepaid_send_fees( + transaction_costs, + &action_receipt.actions, + current_protocol_version, + )?, + )?; let total_gas_cost = safe_gas_to_balance(action_receipt.gas_price, total_gas)?; total_cost = safe_add_balance(total_cost, total_gas_cost)?; } diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index 7688939115c..3c36cf34e1e 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -125,11 +125,8 @@ pub fn total_send_fees( let delegate_action = &signed_delegate_action.delegate_action; let sender_is_receiver = delegate_action.sender_id == delegate_action.receiver_id; - // `total_send_fees` is multiplied 2 because the first fee is charged for sending - // the inner actions to the tx receiver shard, the second fee is charged for sending - // the inner actions from the tx receiver shard to the delegate action receiver shard delegate_cost - + 2 * total_send_fees( + + total_send_fees( config, sender_is_receiver, &delegate_action.get_actions(), @@ -143,6 +140,36 @@ pub fn total_send_fees( Ok(result) } +/// Total sum of gas that needs to be burnt to send the inner actions of DelegateAction +pub fn total_prepaid_send_fees( + config: &RuntimeFeesConfig, + actions: &[Action], + current_protocol_version: ProtocolVersion, +) -> Result { + let mut result = 0; + + for action in actions { + use Action::*; + let delta = match action { + Delegate(signed_delegate_action) => { + let delegate_action = &signed_delegate_action.delegate_action; + let sender_is_receiver = delegate_action.sender_id == delegate_action.receiver_id; + + total_send_fees( + config, + sender_is_receiver, + &delegate_action.get_actions(), + &delegate_action.receiver_id, + current_protocol_version, + )? + } + _ => 0, + }; + result = safe_add_gas(result, delta)?; + } + Ok(result) +} + pub fn exec_fee( config: &RuntimeFeesConfig, action: &Action, @@ -210,7 +237,10 @@ pub fn tx_cost( current_protocol_version, )?, )?; - let prepaid_gas = total_prepaid_gas(&transaction.actions)?; + let prepaid_gas = safe_add_gas( + total_prepaid_gas(&transaction.actions)?, + total_prepaid_send_fees(config, &transaction.actions, current_protocol_version)?, + )?; // If signer is equals to receiver the receipt will be processed at the same block as this // transaction. Otherwise it will processed in the next block and the gas might be inflated. let initial_receipt_hop = if transaction.signer_id == transaction.receiver_id { 0 } else { 1 }; diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index c1d24ac68f2..b4c71cfb928 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -3,6 +3,7 @@ use std::collections::{HashMap, HashSet}; use std::rc::Rc; use std::sync::Arc; +use config::total_prepaid_send_fees; use near_primitives::sandbox::state_patch::SandboxStatePatch; use tracing::debug; @@ -758,7 +759,14 @@ impl Runtime { transaction_costs: &RuntimeFeesConfig, ) -> Result { let total_deposit = total_deposit(&action_receipt.actions)?; - let prepaid_gas = total_prepaid_gas(&action_receipt.actions)?; + let prepaid_gas = safe_add_gas( + total_prepaid_gas(&action_receipt.actions)?, + total_prepaid_send_fees( + transaction_costs, + &action_receipt.actions, + current_protocol_version, + )?, + )?; let prepaid_exec_gas = safe_add_gas( total_prepaid_exec_fees( transaction_costs, From b344298af15a504a4e775d240ef53a63a47b5773 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Wed, 9 Nov 2022 02:27:24 +0200 Subject: [PATCH 12/30] Fixed review issues. Added a test for DelegateAction deserialization --- core/primitives/src/errors.rs | 6 ---- core/primitives/src/transaction.rs | 52 ++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 8c7d9b0f2fd..6557d032ace 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -200,8 +200,6 @@ pub enum ActionsValidationError { UnsuitableStakingKey { public_key: PublicKey }, /// The attached amount of gas in a FunctionCall action has to be a positive number. FunctionCallZeroAttachedGas, - /// The actions in DelegateAction are searilized incorrectly - DelegateActionDeserializeError, /// DelegateAction actions contain another DelegateAction DelegateActionCantContainNestedOne, /// There should be the only one DelegateAction @@ -323,10 +321,6 @@ impl Display for ActionsValidationError { f, "The attached amount of gas in a FunctionCall action has to be a positive number", ), - ActionsValidationError::DelegateActionDeserializeError => write!( - f, - "Can't deserialize actions in DelegateAction" - ), ActionsValidationError::DelegateActionCantContainNestedOne => write!( f, "DelegateAction must not contain another DelegateAction" diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index dd86fe7964c..a4a14517205 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -20,6 +20,8 @@ use near_primitives_core::profile::ProfileData; pub type LogEntry = String; +const ACTION_DELEGATE_NUMBER: u8 = 8; + #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] #[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] pub struct Transaction { @@ -72,6 +74,7 @@ pub enum Action { AddKey(AddKeyAction), DeleteKey(DeleteKeyAction), DeleteAccount(DeleteAccountAction), + /// If Action::Delegate is moved, need to change the value of ACTION_DELEGATE_NUMBER constant Delegate(SignedDelegateAction), } @@ -227,16 +230,20 @@ impl From for Action { #[derive(Serialize, BorshSerialize, Deserialize, PartialEq, Eq, Clone, Debug)] pub struct NonDelegateAction(Action); -const ACTION_DELEGATE_NUMBER: u8 = 8; - -impl From<&NonDelegateAction> for Action { - fn from(action: &NonDelegateAction) -> Self { - action.0.clone() +impl From for Action { + fn from(action: NonDelegateAction) -> Self { + action.0 } } impl borsh::de::BorshDeserialize for NonDelegateAction { fn deserialize(buf: &mut &[u8]) -> ::core::result::Result { + if buf.is_empty() { + return Err(Error::new( + ErrorKind::InvalidInput, + "Failed to deserialize DelegateAction", + )); + } match buf[0] { ACTION_DELEGATE_NUMBER => Err(Error::new( ErrorKind::InvalidInput, @@ -289,7 +296,7 @@ impl From for Action { impl DelegateAction { pub fn get_actions(&self) -> Vec { - self.actions.iter().map(|a| a.into()).collect() + self.actions.iter().map(|a| a.clone().into()).collect() } pub fn get_hash(&self) -> CryptoHash { @@ -633,4 +640,37 @@ mod tests { let hashes = outcome.to_hashes(); assert_eq!(hashes.len(), 3); } + + fn create_delegate_action(actions: Vec) -> Action { + Action::Delegate(SignedDelegateAction { + delegate_action: DelegateAction { + sender_id: "aaa".parse().unwrap(), + receiver_id: "bbb".parse().unwrap(), + actions: actions.iter().map(|a| NonDelegateAction(a.clone())).collect(), + nonce: 1, + max_block_height: 2, + public_key: PublicKey::empty(KeyType::ED25519), + }, + signature: Signature::empty(KeyType::ED25519), + }) + } + + #[test] + fn test_delegate_action_deserialization() { + NonDelegateAction::try_from_slice(Vec::new().as_ref()).expect_err("Expected an error. Buffer is empty"); + + let delegate_action = create_delegate_action(Vec::::new()); + let serialized_non_delegate_action = create_delegate_action(vec!{delegate_action}) + .try_to_vec() + .expect("Expect ok"); + + Action::try_from_slice(&serialized_non_delegate_action) + .expect_err("Expected a nested DelegateAction error"); + + let serialized_delegate_action = create_delegate_action(vec!{Action::CreateAccount(CreateAccountAction {})}) + .try_to_vec() + .expect("Expect ok"); + Action::try_from_slice(&serialized_delegate_action) + .expect("Expected ok"); + } } From d6ef8ba12af8108fa4ea6912d7fbb20f86d45301 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Wed, 9 Nov 2022 12:31:29 +0200 Subject: [PATCH 13/30] Use `signer_public_key` instead of `delegate_action.public_key` in a new receipt --- runtime/runtime/src/actions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 0afb4e1f79f..d86e72283ee 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -664,7 +664,7 @@ pub(crate) fn apply_delegate_action( sender_id, &delegate_action.receiver_id, &actions, - &delegate_action.public_key, + &action_receipt.signer_public_key, action_receipt.gas_price, ); From df86e3a25413859ae2814998f38d687c631e1916 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 10 Nov 2022 23:45:51 +0200 Subject: [PATCH 14/30] Add tests to actions.rs. Fix tests in transaction.rs --- core/primitives/src/transaction.rs | 37 +- runtime/runtime/src/actions.rs | 612 ++++++++++++++++++++++++++++- 2 files changed, 629 insertions(+), 20 deletions(-) diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index a4a14517205..c68e1429dd7 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -228,7 +228,7 @@ impl From for Action { #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] #[derive(Serialize, BorshSerialize, Deserialize, PartialEq, Eq, Clone, Debug)] -pub struct NonDelegateAction(Action); +pub struct NonDelegateAction(pub Action); impl From for Action { fn from(action: NonDelegateAction) -> Self { @@ -657,20 +657,29 @@ mod tests { #[test] fn test_delegate_action_deserialization() { - NonDelegateAction::try_from_slice(Vec::new().as_ref()).expect_err("Expected an error. Buffer is empty"); + // Expected an error. Buffer is empty + assert_eq!( + NonDelegateAction::try_from_slice(Vec::new().as_ref()).map_err(|e| e.kind()), + Err(ErrorKind::InvalidInput) + ); let delegate_action = create_delegate_action(Vec::::new()); - let serialized_non_delegate_action = create_delegate_action(vec!{delegate_action}) - .try_to_vec() - .expect("Expect ok"); - - Action::try_from_slice(&serialized_non_delegate_action) - .expect_err("Expected a nested DelegateAction error"); - - let serialized_delegate_action = create_delegate_action(vec!{Action::CreateAccount(CreateAccountAction {})}) - .try_to_vec() - .expect("Expect ok"); - Action::try_from_slice(&serialized_delegate_action) - .expect("Expected ok"); + let serialized_non_delegate_action = + create_delegate_action(vec![delegate_action]).try_to_vec().expect("Expect ok"); + + // Expected a nested DelegateAction error + assert_eq!( + NonDelegateAction::try_from_slice(&serialized_non_delegate_action) + .map_err(|e| e.kind()), + Err(ErrorKind::InvalidInput) + ); + + let serialized_delegate_action = + create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]) + .try_to_vec() + .expect("Expect ok"); + + // Valid action + Action::try_from_slice(&serialized_delegate_action).expect("Expected ok"); } } diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index d86e72283ee..09188f611be 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -652,7 +652,7 @@ pub(crate) fn apply_delegate_action( return Ok(()); }; - validate_delegate_action_key(state_update, apply_state, sender_id, delegate_action, result)?; + validate_delegate_action_key(state_update, apply_state, delegate_action, result)?; if result.result.is_err() { return Ok(()); } @@ -708,17 +708,20 @@ fn receipt_required_gas(apply_state: &ApplyState, receipt: &Receipt) -> Result Result<(), RuntimeError> { - let mut access_key = match get_access_key(state_update, sender_id, &delegate_action.public_key)? - { + // 'delegate_action.sender_id' account existence must be checked by a caller + let mut access_key = match get_access_key( + state_update, + &delegate_action.sender_id, + &delegate_action.public_key, + )? { Some(access_key) => access_key, None => { result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( InvalidAccessKeyError::AccessKeyNotFound { - account_id: sender_id.clone(), + account_id: delegate_action.sender_id.clone(), public_key: delegate_action.public_key.clone(), }, ) @@ -801,7 +804,7 @@ fn validate_delegate_action_key( set_access_key( state_update, - sender_id.clone(), + delegate_action.sender_id.clone(), delegate_action.public_key.clone(), &access_key, ); @@ -926,8 +929,15 @@ pub(crate) fn check_account_existence( #[cfg(test)] mod tests { + use std::sync::Arc; + + use near_primitives::account::FunctionCallPermission; use near_primitives::hash::hash; + use near_primitives::runtime::migration_data::MigrationFlags; + use near_primitives::transaction::{CreateAccountAction, NonDelegateAction}; use near_primitives::trie_key::TrieKey; + use near_primitives::types::{EpochId, StateChangeCause}; + use near_store::set_account; use near_store::test_utils::create_tries; use super::*; @@ -1110,4 +1120,594 @@ mod tests { }) ); } + + fn create_delegate_action_receipt() -> (ActionReceipt, SignedDelegateAction) { + let signed_delegate_action = SignedDelegateAction { + delegate_action: DelegateAction { + sender_id: "bob.test.near".parse().unwrap(), + receiver_id: "token.test.near".parse().unwrap(), + actions: vec![ + NonDelegateAction( + Action::FunctionCall( + FunctionCallAction { + method_name: "ft_transfer".parse().unwrap(), + args: vec![123, 34, 114, 101, 99, 101, 105, 118, 101, 114, 95, 105, 100, 34, 58, 34, 106, 97, 110, 101, 46, 116, 101, 115, 116, 46, 110, 101, 97, 114, 34, 44, 34, 97, 109, 111, 117, 110, 116, 34, 58, 34, 52, 34, 125], + gas: 30000000000000, + deposit: 1, + } + ) + ) + ], + nonce: 19000001, + max_block_height: 57, + public_key: "ed25519:HaYUbyeiNRnyHtQceRgT3gyMBigZFEW9EYYU1KTHtdR1".parse::().unwrap(), + }, + signature: "ed25519:2b1NHmrj7LVgA5H9aDtQmd6JgZqy4nPAYHtNQc88PiEY3xMjpkKMDN1wVWZaXMGx9tjWbXzp4jXSCyTPqUfPdRUB".parse().unwrap() + }; + + let action_receipt = ActionReceipt { + signer_id: "alice.test.near".parse().unwrap(), + signer_public_key: PublicKey::empty(near_crypto::KeyType::ED25519), + gas_price: 1, + output_data_receivers: Vec::new(), + input_data_ids: Vec::new(), + actions: vec![Action::Delegate(signed_delegate_action.clone())], + }; + + (action_receipt, signed_delegate_action) + } + + fn create_apply_state(block_index: BlockHeight) -> ApplyState { + ApplyState { + block_index, + prev_block_hash: CryptoHash::default(), + block_hash: CryptoHash::default(), + epoch_id: EpochId::default(), + epoch_height: 3, + gas_price: 2, + block_timestamp: 1, + gas_limit: None, + random_seed: CryptoHash::default(), + current_protocol_version: 1, + config: Arc::new(RuntimeConfig::test()), + cache: None, + is_new_chunk: false, + migration_data: Arc::default(), + migration_flags: MigrationFlags::default(), + } + } + + fn setup_account( + account_id: &AccountId, + public_key: &PublicKey, + access_key: &AccessKey, + ) -> (TrieUpdate, Account) { + let tries = create_tries(); + let mut state_update = + tries.new_trie_update(ShardUId::single_shard(), CryptoHash::default()); + let account = Account::new(100, 0, CryptoHash::default(), 100); + set_account(&mut state_update, account_id.clone(), &account); + set_access_key(&mut state_update, account_id.clone(), public_key.clone(), access_key); + + state_update.commit(StateChangeCause::InitialState); + let trie_changes = state_update.finalize().unwrap().0; + let (store_update, root) = tries.apply_all(&trie_changes, ShardUId::single_shard()); + store_update.commit().unwrap(); + + (tries.new_trie_update(ShardUId::single_shard(), root), account) + } + + #[test] + fn test_delegate_action() { + let mut result = ActionResult::default(); + let (action_receipt, signed_delegate_action) = create_delegate_action_receipt(); + let sender_id = signed_delegate_action.delegate_action.sender_id.clone(); + let sender_pub_key = signed_delegate_action.delegate_action.public_key.clone(); + let access_key = AccessKey { nonce: 19000000, permission: AccessKeyPermission::FullAccess }; + + let apply_state = + create_apply_state(signed_delegate_action.delegate_action.max_block_height); + let (mut state_update, sender) = setup_account(&sender_id, &sender_pub_key, &access_key); + + apply_delegate_action( + &mut state_update, + &apply_state, + &action_receipt, + &mut Some(sender), + &sender_id, + &signed_delegate_action, + &mut result, + ) + .expect("Expect ok"); + + assert!(result.result.is_ok(), "Result error: {:?}", result.result.err()); + assert_eq!( + result.new_receipts, + vec![Receipt { + predecessor_id: sender_id.clone(), + receiver_id: signed_delegate_action.delegate_action.receiver_id.clone(), + receipt_id: CryptoHash::default(), + receipt: ReceiptEnum::Action(ActionReceipt { + signer_id: action_receipt.signer_id.clone(), + signer_public_key: action_receipt.signer_public_key.clone(), + gas_price: action_receipt.gas_price, + output_data_receivers: Vec::new(), + input_data_ids: Vec::new(), + actions: signed_delegate_action.delegate_action.get_actions(), + }) + }] + ); + } + + #[test] + fn test_delegate_action_signature_verification() { + let mut result = ActionResult::default(); + let (action_receipt, mut signed_delegate_action) = create_delegate_action_receipt(); + let sender_id = signed_delegate_action.delegate_action.sender_id.clone(); + let sender_pub_key = signed_delegate_action.delegate_action.public_key.clone(); + let access_key = AccessKey { nonce: 19000000, permission: AccessKeyPermission::FullAccess }; + + let apply_state = + create_apply_state(signed_delegate_action.delegate_action.max_block_height); + let (mut state_update, sender) = setup_account(&sender_id, &sender_pub_key, &access_key); + + // Corrupt receiver_id. Signature verifycation must fail. + signed_delegate_action.delegate_action.receiver_id = "www.test.near".parse().unwrap(); + + apply_delegate_action( + &mut state_update, + &apply_state, + &action_receipt, + &mut Some(sender), + &sender_id, + &signed_delegate_action, + &mut result, + ) + .expect("Expect ok"); + + assert_eq!(result.result, Err(ActionErrorKind::DelegateActionInvalidSignature.into())); + } + + #[test] + fn test_delegate_action_max_height() { + let mut result = ActionResult::default(); + let (action_receipt, signed_delegate_action) = create_delegate_action_receipt(); + let sender_id = signed_delegate_action.delegate_action.sender_id.clone(); + let sender_pub_key = signed_delegate_action.delegate_action.public_key.clone(); + let access_key = AccessKey { nonce: 19000000, permission: AccessKeyPermission::FullAccess }; + + // Setup current block as higher than max_block_height. Must fail. + let apply_state = + create_apply_state(signed_delegate_action.delegate_action.max_block_height + 1); + let (mut state_update, sender) = setup_account(&sender_id, &sender_pub_key, &access_key); + + apply_delegate_action( + &mut state_update, + &apply_state, + &action_receipt, + &mut Some(sender), + &sender_id, + &signed_delegate_action, + &mut result, + ) + .expect("Expect ok"); + + assert_eq!(result.result, Err(ActionErrorKind::DelegateActionExpired.into())); + } + + #[test] + fn test_delegate_action_validate_sender_account() { + let mut result = ActionResult::default(); + let (action_receipt, signed_delegate_action) = create_delegate_action_receipt(); + let sender_id = signed_delegate_action.delegate_action.sender_id.clone(); + let sender_pub_key = signed_delegate_action.delegate_action.public_key.clone(); + let access_key = AccessKey { nonce: 19000000, permission: AccessKeyPermission::FullAccess }; + + let apply_state = + create_apply_state(signed_delegate_action.delegate_action.max_block_height); + let (mut state_update, sender) = setup_account(&sender_id, &sender_pub_key, &access_key); + + // Use a different sender_id. Must fail. + apply_delegate_action( + &mut state_update, + &apply_state, + &action_receipt, + &mut Some(sender), + &"www.test.near".parse().unwrap(), + &signed_delegate_action, + &mut result, + ) + .expect("Expect ok"); + + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionSenderDoesNotMatchTxReceiver { + sender_id: sender_id.clone(), + receiver_id: "www.test.near".parse().unwrap(), + } + .into()) + ); + + result = ActionResult::default(); + + // Sender account doesn't exist. Must fail. + apply_delegate_action( + &mut state_update, + &apply_state, + &action_receipt, + &mut None, + &sender_id, + &signed_delegate_action, + &mut result, + ) + .expect("Expect ok"); + + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionSenderDoesNotExist { sender_id: sender_id.clone() } + .into()) + ); + } + + #[test] + fn test_validate_delegate_action_key_update_nonce() { + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let sender_id = signed_delegate_action.delegate_action.sender_id.clone(); + let sender_pub_key = signed_delegate_action.delegate_action.public_key.clone(); + let access_key = AccessKey { nonce: 19000000, permission: AccessKeyPermission::FullAccess }; + + let apply_state = + create_apply_state(signed_delegate_action.delegate_action.max_block_height); + let (mut state_update, _) = setup_account(&sender_id, &sender_pub_key, &access_key); + + // Everything is ok + let mut result = ActionResult::default(); + validate_delegate_action_key( + &mut state_update, + &apply_state, + &signed_delegate_action.delegate_action, + &mut result, + ) + .expect("Expect ok"); + assert!(result.result.is_ok(), "Result error: {:?}", result.result); + + // Must fail, Nonce had been updated by previous step. + result = ActionResult::default(); + validate_delegate_action_key( + &mut state_update, + &apply_state, + &signed_delegate_action.delegate_action, + &mut result, + ) + .expect("Expect ok"); + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionInvalidNonce { + delegate_nonce: signed_delegate_action.delegate_action.nonce, + ak_nonce: signed_delegate_action.delegate_action.nonce, + } + .into()) + ); + + // Increment nonce. Must pass. + result = ActionResult::default(); + let mut delegate_action = signed_delegate_action.delegate_action.clone(); + delegate_action.nonce += 1; + validate_delegate_action_key( + &mut state_update, + &apply_state, + &delegate_action, + &mut result, + ) + .expect("Expect ok"); + assert!(result.result.is_ok(), "Result error: {:?}", result.result); + } + + #[test] + fn test_delegate_action_key_doesnt_exist() { + let mut result = ActionResult::default(); + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let sender_id = signed_delegate_action.delegate_action.sender_id.clone(); + let sender_pub_key = signed_delegate_action.delegate_action.public_key.clone(); + let access_key = AccessKey { nonce: 19000000, permission: AccessKeyPermission::FullAccess }; + + let apply_state = + create_apply_state(signed_delegate_action.delegate_action.max_block_height); + let (mut state_update, _) = setup_account( + &sender_id, + &PublicKey::empty(near_crypto::KeyType::ED25519), + &access_key, + ); + + validate_delegate_action_key( + &mut state_update, + &apply_state, + &signed_delegate_action.delegate_action, + &mut result, + ) + .expect("Expect ok"); + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::AccessKeyNotFound { + account_id: sender_id.clone(), + public_key: sender_pub_key.clone(), + }, + ) + .into()) + ); + } + + #[test] + fn test_delegate_action_key_incorrect_nonce() { + let mut result = ActionResult::default(); + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let sender_id = signed_delegate_action.delegate_action.sender_id.clone(); + let sender_pub_key = signed_delegate_action.delegate_action.public_key.clone(); + let access_key = AccessKey { + nonce: signed_delegate_action.delegate_action.nonce, + permission: AccessKeyPermission::FullAccess, + }; + + let apply_state = + create_apply_state(signed_delegate_action.delegate_action.max_block_height); + let (mut state_update, _) = setup_account(&sender_id, &sender_pub_key, &access_key); + + validate_delegate_action_key( + &mut state_update, + &apply_state, + &signed_delegate_action.delegate_action, + &mut result, + ) + .expect("Expect ok"); + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionInvalidNonce { + delegate_nonce: signed_delegate_action.delegate_action.nonce, + ak_nonce: signed_delegate_action.delegate_action.nonce, + } + .into()) + ); + } + + #[test] + fn test_delegate_action_key_nonce_too_large() { + let mut result = ActionResult::default(); + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let sender_id = signed_delegate_action.delegate_action.sender_id.clone(); + let sender_pub_key = signed_delegate_action.delegate_action.public_key.clone(); + let access_key = AccessKey { nonce: 19000000, permission: AccessKeyPermission::FullAccess }; + + let apply_state = create_apply_state(1); + let (mut state_update, _) = setup_account(&sender_id, &sender_pub_key, &access_key); + + validate_delegate_action_key( + &mut state_update, + &apply_state, + &signed_delegate_action.delegate_action, + &mut result, + ) + .expect("Expect ok"); + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionNonceTooLarge { + delegate_nonce: signed_delegate_action.delegate_action.nonce, + upper_bound: 1000000, + } + .into()) + ); + } + + fn test_delegate_action_key_permissions( + access_key: &AccessKey, + delegate_action: &DelegateAction, + ) -> ActionResult { + let mut result = ActionResult::default(); + let sender_id = delegate_action.sender_id.clone(); + let sender_pub_key = delegate_action.public_key.clone(); + + let apply_state = create_apply_state(delegate_action.max_block_height); + let (mut state_update, _) = setup_account(&sender_id, &sender_pub_key, &access_key); + + validate_delegate_action_key( + &mut state_update, + &apply_state, + &delegate_action, + &mut result, + ) + .expect("Expect ok"); + + result + } + + #[test] + fn test_delegate_action_key_permissions_fncall() { + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let access_key = AccessKey { + nonce: 19000000, + permission: AccessKeyPermission::FunctionCall(FunctionCallPermission { + allowance: None, + receiver_id: signed_delegate_action.delegate_action.receiver_id.to_string(), + method_names: vec!["test_method".parse().unwrap()], + }), + }; + + let mut delegate_action = signed_delegate_action.delegate_action.clone(); + delegate_action.actions = + vec![NonDelegateAction(Action::FunctionCall(FunctionCallAction { + args: Vec::new(), + deposit: 0, + gas: 300, + method_name: "test_method".parse().unwrap(), + }))]; + let result = test_delegate_action_key_permissions(&access_key, &delegate_action); + assert!(result.result.is_ok(), "Result error {:?}", result.result); + } + + #[test] + fn test_delegate_action_key_permissions_incorrect_action() { + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let access_key = AccessKey { + nonce: 19000000, + permission: AccessKeyPermission::FunctionCall(FunctionCallPermission { + allowance: None, + receiver_id: signed_delegate_action.delegate_action.receiver_id.to_string(), + method_names: vec!["test_method".parse().unwrap()], + }), + }; + + let mut delegate_action = signed_delegate_action.delegate_action.clone(); + delegate_action.actions = + vec![NonDelegateAction(Action::CreateAccount(CreateAccountAction {}))]; + + let result = test_delegate_action_key_permissions(&access_key, &delegate_action); + + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::RequiresFullAccess, + ) + .into()) + ); + } + + #[test] + fn test_delegate_action_key_permissions_actions_number() { + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let access_key = AccessKey { + nonce: 19000000, + permission: AccessKeyPermission::FunctionCall(FunctionCallPermission { + allowance: None, + receiver_id: signed_delegate_action.delegate_action.receiver_id.to_string(), + method_names: vec!["test_method".parse().unwrap()], + }), + }; + + let mut delegate_action = signed_delegate_action.delegate_action.clone(); + delegate_action.actions = vec![ + NonDelegateAction(Action::FunctionCall(FunctionCallAction { + args: Vec::new(), + deposit: 0, + gas: 300, + method_name: "test_method".parse().unwrap(), + })), + NonDelegateAction(Action::FunctionCall(FunctionCallAction { + args: Vec::new(), + deposit: 0, + gas: 300, + method_name: "test_method".parse().unwrap(), + })), + ]; + + let result = test_delegate_action_key_permissions(&access_key, &delegate_action); + + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::RequiresFullAccess, + ) + .into()) + ); + } + + #[test] + fn test_delegate_action_key_permissions_fncall_deposit() { + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let access_key = AccessKey { + nonce: 19000000, + permission: AccessKeyPermission::FunctionCall(FunctionCallPermission { + allowance: None, + receiver_id: signed_delegate_action.delegate_action.receiver_id.to_string(), + method_names: Vec::new(), + }), + }; + + let mut delegate_action = signed_delegate_action.delegate_action.clone(); + delegate_action.actions = + vec![NonDelegateAction(Action::FunctionCall(FunctionCallAction { + args: Vec::new(), + deposit: 1, + gas: 300, + method_name: "test_method".parse().unwrap(), + }))]; + + let result = test_delegate_action_key_permissions(&access_key, &delegate_action); + + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::DepositWithFunctionCall, + ) + .into()) + ); + } + + #[test] + fn test_delegate_action_key_permissions_receiver_id() { + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let access_key = AccessKey { + nonce: 19000000, + permission: AccessKeyPermission::FunctionCall(FunctionCallPermission { + allowance: None, + receiver_id: "another.near".parse().unwrap(), + method_names: Vec::new(), + }), + }; + + let mut delegate_action = signed_delegate_action.delegate_action.clone(); + delegate_action.actions = + vec![NonDelegateAction(Action::FunctionCall(FunctionCallAction { + args: Vec::new(), + deposit: 0, + gas: 300, + method_name: "test_method".parse().unwrap(), + }))]; + + let result = test_delegate_action_key_permissions(&access_key, &delegate_action); + + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::ReceiverMismatch { + tx_receiver: delegate_action.receiver_id.clone(), + ak_receiver: "another.near".parse().unwrap(), + }, + ) + .into()) + ); + } + + #[test] + fn test_delegate_action_key_permissions_method() { + let (_, signed_delegate_action) = create_delegate_action_receipt(); + let access_key = AccessKey { + nonce: 19000000, + permission: AccessKeyPermission::FunctionCall(FunctionCallPermission { + allowance: None, + receiver_id: signed_delegate_action.delegate_action.receiver_id.to_string(), + method_names: vec!["another_method".parse().unwrap()], + }), + }; + + let mut delegate_action = signed_delegate_action.delegate_action.clone(); + delegate_action.actions = + vec![NonDelegateAction(Action::FunctionCall(FunctionCallAction { + args: Vec::new(), + deposit: 0, + gas: 300, + method_name: "test_method".parse().unwrap(), + }))]; + + let result = test_delegate_action_key_permissions(&access_key, &delegate_action); + + assert_eq!( + result.result, + Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::MethodNameMismatch { + method_name: "test_method".parse().unwrap(), + }, + ) + .into()) + ); + } } From 59a5986a969d90fe3cf6fdfdb2ba10e74b92b78a Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 10 Nov 2022 23:52:38 +0200 Subject: [PATCH 15/30] Return `AccountDoesNotExist` error if `sender` doesn't exist --- core/primitives/src/errors.rs | 3 -- core/primitives/src/transaction.rs | 2 +- runtime/runtime/src/actions.rs | 62 +++++++++++------------------- runtime/runtime/src/lib.rs | 1 - 4 files changed, 23 insertions(+), 45 deletions(-) diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 6557d032ace..c8c205100d4 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -457,8 +457,6 @@ pub enum ActionErrorKind { DelegateActionInvalidSignature, /// Receiver of the transaction doesn't match Sender of the delegate action DelegateActionSenderDoesNotMatchTxReceiver { sender_id: AccountId, receiver_id: AccountId }, - /// Sender account doesn't exist - DelegateActionSenderDoesNotExist { sender_id: AccountId }, /// Delegate action has expired. `max_block_height` is less than actual block height. DelegateActionExpired, /// The given public key doesn't exist for Sender account @@ -779,7 +777,6 @@ impl Display for ActionErrorKind { ActionErrorKind::DeleteAccountWithLargeState { account_id } => write!(f, "The state of account {} is too large and therefore cannot be deleted", account_id), ActionErrorKind::DelegateActionInvalidSignature => write!(f, "DelegateAction is not signed with the given public key"), ActionErrorKind::DelegateActionSenderDoesNotMatchTxReceiver { sender_id, receiver_id } => write!(f, "Transaction reciever {} doesn't match DelegateAction sender {}", sender_id, receiver_id), - ActionErrorKind::DelegateActionSenderDoesNotExist { sender_id } => write!(f, "DelegateAction sender account {} doesn't exist", sender_id), ActionErrorKind::DelegateActionExpired => write!(f, "DelegateAction has expired"), ActionErrorKind::DelegateActionAccessKeyError(access_key_error) => Display::fmt(&access_key_error, f), ActionErrorKind::DelegateActionInvalidNonce { delegate_nonce, ak_nonce } => write!(f, "DelegateAction nonce {} must be larger than nonce of the used access key {}", delegate_nonce, ak_nonce), diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index c68e1429dd7..ceec93037df 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -679,7 +679,7 @@ mod tests { .try_to_vec() .expect("Expect ok"); - // Valid action + // Valid action Action::try_from_slice(&serialized_delegate_action).expect("Expected ok"); } } diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 09188f611be..549ebfd6b7e 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -622,7 +622,6 @@ pub(crate) fn apply_delegate_action( state_update: &mut TrieUpdate, apply_state: &ApplyState, action_receipt: &ActionReceipt, - sender: &mut Option, sender_id: &AccountId, signed_delegate_action: &SignedDelegateAction, result: &mut ActionResult, @@ -645,12 +644,6 @@ pub(crate) fn apply_delegate_action( .into()); return Ok(()); } - if sender.is_none() { - result.result = - Err(ActionErrorKind::DelegateActionSenderDoesNotExist { sender_id: sender_id.clone() } - .into()); - return Ok(()); - }; validate_delegate_action_key(state_update, apply_state, delegate_action, result)?; if result.result.is_err() { @@ -914,7 +907,8 @@ pub(crate) fn check_account_existence( | Action::Stake(_) | Action::AddKey(_) | Action::DeleteKey(_) - | Action::DeleteAccount(_) => { + | Action::DeleteAccount(_) + | Action::Delegate(_) => { if account.is_none() { return Err(ActionErrorKind::AccountDoesNotExist { account_id: account_id.clone(), @@ -922,7 +916,6 @@ pub(crate) fn check_account_existence( .into()); } } - Action::Delegate(_) => (), }; Ok(()) } @@ -1181,7 +1174,7 @@ mod tests { account_id: &AccountId, public_key: &PublicKey, access_key: &AccessKey, - ) -> (TrieUpdate, Account) { + ) -> TrieUpdate { let tries = create_tries(); let mut state_update = tries.new_trie_update(ShardUId::single_shard(), CryptoHash::default()); @@ -1194,7 +1187,7 @@ mod tests { let (store_update, root) = tries.apply_all(&trie_changes, ShardUId::single_shard()); store_update.commit().unwrap(); - (tries.new_trie_update(ShardUId::single_shard(), root), account) + tries.new_trie_update(ShardUId::single_shard(), root) } #[test] @@ -1207,13 +1200,12 @@ mod tests { let apply_state = create_apply_state(signed_delegate_action.delegate_action.max_block_height); - let (mut state_update, sender) = setup_account(&sender_id, &sender_pub_key, &access_key); + let mut state_update = setup_account(&sender_id, &sender_pub_key, &access_key); apply_delegate_action( &mut state_update, &apply_state, &action_receipt, - &mut Some(sender), &sender_id, &signed_delegate_action, &mut result, @@ -1249,7 +1241,7 @@ mod tests { let apply_state = create_apply_state(signed_delegate_action.delegate_action.max_block_height); - let (mut state_update, sender) = setup_account(&sender_id, &sender_pub_key, &access_key); + let mut state_update = setup_account(&sender_id, &sender_pub_key, &access_key); // Corrupt receiver_id. Signature verifycation must fail. signed_delegate_action.delegate_action.receiver_id = "www.test.near".parse().unwrap(); @@ -1258,7 +1250,6 @@ mod tests { &mut state_update, &apply_state, &action_receipt, - &mut Some(sender), &sender_id, &signed_delegate_action, &mut result, @@ -1279,13 +1270,12 @@ mod tests { // Setup current block as higher than max_block_height. Must fail. let apply_state = create_apply_state(signed_delegate_action.delegate_action.max_block_height + 1); - let (mut state_update, sender) = setup_account(&sender_id, &sender_pub_key, &access_key); + let mut state_update = setup_account(&sender_id, &sender_pub_key, &access_key); apply_delegate_action( &mut state_update, &apply_state, &action_receipt, - &mut Some(sender), &sender_id, &signed_delegate_action, &mut result, @@ -1305,14 +1295,13 @@ mod tests { let apply_state = create_apply_state(signed_delegate_action.delegate_action.max_block_height); - let (mut state_update, sender) = setup_account(&sender_id, &sender_pub_key, &access_key); + let mut state_update = setup_account(&sender_id, &sender_pub_key, &access_key); // Use a different sender_id. Must fail. apply_delegate_action( &mut state_update, &apply_state, &action_receipt, - &mut Some(sender), &"www.test.near".parse().unwrap(), &signed_delegate_action, &mut result, @@ -1328,24 +1317,17 @@ mod tests { .into()) ); - result = ActionResult::default(); - // Sender account doesn't exist. Must fail. - apply_delegate_action( - &mut state_update, - &apply_state, - &action_receipt, - &mut None, - &sender_id, - &signed_delegate_action, - &mut result, - ) - .expect("Expect ok"); - assert_eq!( - result.result, - Err(ActionErrorKind::DelegateActionSenderDoesNotExist { sender_id: sender_id.clone() } - .into()) + check_account_existence( + &Action::Delegate(signed_delegate_action), + &mut None, + &sender_id, + 1, + false, + false + ), + Err(ActionErrorKind::AccountDoesNotExist { account_id: sender_id.clone() }.into()) ); } @@ -1358,7 +1340,7 @@ mod tests { let apply_state = create_apply_state(signed_delegate_action.delegate_action.max_block_height); - let (mut state_update, _) = setup_account(&sender_id, &sender_pub_key, &access_key); + let mut state_update = setup_account(&sender_id, &sender_pub_key, &access_key); // Everything is ok let mut result = ActionResult::default(); @@ -1413,7 +1395,7 @@ mod tests { let apply_state = create_apply_state(signed_delegate_action.delegate_action.max_block_height); - let (mut state_update, _) = setup_account( + let mut state_update = setup_account( &sender_id, &PublicKey::empty(near_crypto::KeyType::ED25519), &access_key, @@ -1451,7 +1433,7 @@ mod tests { let apply_state = create_apply_state(signed_delegate_action.delegate_action.max_block_height); - let (mut state_update, _) = setup_account(&sender_id, &sender_pub_key, &access_key); + let mut state_update = setup_account(&sender_id, &sender_pub_key, &access_key); validate_delegate_action_key( &mut state_update, @@ -1479,7 +1461,7 @@ mod tests { let access_key = AccessKey { nonce: 19000000, permission: AccessKeyPermission::FullAccess }; let apply_state = create_apply_state(1); - let (mut state_update, _) = setup_account(&sender_id, &sender_pub_key, &access_key); + let mut state_update = setup_account(&sender_id, &sender_pub_key, &access_key); validate_delegate_action_key( &mut state_update, @@ -1507,7 +1489,7 @@ mod tests { let sender_pub_key = delegate_action.public_key.clone(); let apply_state = create_apply_state(delegate_action.max_block_height); - let (mut state_update, _) = setup_account(&sender_id, &sender_pub_key, &access_key); + let mut state_update = setup_account(&sender_id, &sender_pub_key, &access_key); validate_delegate_action_key( &mut state_update, diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index b4c71cfb928..17d42b371b0 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -448,7 +448,6 @@ impl Runtime { state_update, apply_state, action_receipt, - account, account_id, signed_delegate_action, &mut result, From cd289d4fd00e7fd4849208c77b0c9080b296095b Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Fri, 11 Nov 2022 00:43:14 +0200 Subject: [PATCH 16/30] Fix review issues --- core/primitives/src/transaction.rs | 14 ++++++++------ runtime/runtime/src/config.rs | 1 - 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index ceec93037df..8a42bcba20f 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -20,6 +20,7 @@ use near_primitives_core::profile::ProfileData; pub type LogEntry = String; +// This is an index number of Action::Delegate in Action enumeration const ACTION_DELEGATE_NUMBER: u8 = 8; #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] @@ -74,7 +75,6 @@ pub enum Action { AddKey(AddKeyAction), DeleteKey(DeleteKeyAction), DeleteAccount(DeleteAccountAction), - /// If Action::Delegate is moved, need to change the value of ACTION_DELEGATE_NUMBER constant Delegate(SignedDelegateAction), } @@ -674,12 +674,14 @@ mod tests { Err(ErrorKind::InvalidInput) ); - let serialized_delegate_action = - create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]) - .try_to_vec() - .expect("Expect ok"); + let delegate_action = + create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]); + let serialized_delegate_action = delegate_action.try_to_vec().expect("Expect ok"); // Valid action - Action::try_from_slice(&serialized_delegate_action).expect("Expected ok"); + assert_eq!( + Action::try_from_slice(&serialized_delegate_action).expect("Expect ok"), + delegate_action + ); } } diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index 3c36cf34e1e..bd9023ae676 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -123,7 +123,6 @@ pub fn total_send_fees( Delegate(signed_delegate_action) => { let delegate_cost = cfg.delegate_cost.send_fee(sender_is_receiver); let delegate_action = &signed_delegate_action.delegate_action; - let sender_is_receiver = delegate_action.sender_id == delegate_action.receiver_id; delegate_cost + total_send_fees( From 9a9c0dad4580d01ed5c1f153f5151c9213c997e9 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 17 Nov 2022 02:14:00 +0200 Subject: [PATCH 17/30] Add protocol_feature_delegate_action The feature flag enables DelegateAction support --- core/primitives/Cargo.toml | 2 ++ core/primitives/src/transaction.rs | 13 ++++++++++++- core/primitives/src/version.rs | 4 ++++ nearcore/Cargo.toml | 5 +++++ neard/Cargo.toml | 1 + runtime/runtime/Cargo.toml | 1 + 6 files changed, 25 insertions(+), 1 deletion(-) diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index 6a7ef67a0e2..29cec3bdded 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -45,11 +45,13 @@ dump_errors_schema = ["near-rpc-error-macro/dump_errors_schema"] protocol_feature_fix_staking_threshold = [] protocol_feature_fix_contract_loading_cost = [] protocol_feature_account_id_in_function_call_permission = [] +protocol_feature_delegate_action = [] nightly = [ "nightly_protocol", "protocol_feature_fix_staking_threshold", "protocol_feature_fix_contract_loading_cost", "protocol_feature_account_id_in_function_call_permission", + "protocol_feature_delegate_action", ] nightly_protocol = [] diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 8a42bcba20f..e2a7841132a 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -272,12 +272,23 @@ pub struct DelegateAction { pub public_key: PublicKey, } #[cfg_attr(feature = "deepsize_feature", derive(deepsize::DeepSizeOf))] -#[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] +#[cfg_attr(feature = "protocol_feature_delegate_action", derive(BorshDeserialize))] +#[derive(BorshSerialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] pub struct SignedDelegateAction { pub delegate_action: DelegateAction, pub signature: Signature, } +#[cfg(not(feature = "protocol_feature_delegate_action"))] +impl borsh::de::BorshDeserialize for SignedDelegateAction { + fn deserialize(_buf: &mut &[u8]) -> ::core::result::Result { + return Err(Error::new( + ErrorKind::InvalidInput, + "Delegate action isn't supported", + )); + } +} + impl SignedDelegateAction { pub fn verify(&self) -> bool { let delegate_action = &self.delegate_action; diff --git a/core/primitives/src/version.rs b/core/primitives/src/version.rs index 0ca3c2d4520..d5965dadf02 100644 --- a/core/primitives/src/version.rs +++ b/core/primitives/src/version.rs @@ -168,6 +168,8 @@ pub enum ProtocolFeature { /// Validate account id for function call access keys. #[cfg(feature = "protocol_feature_account_id_in_function_call_permission")] AccountIdInFunctionCallPermission, + #[cfg(feature = "protocol_feature_delegate_action")] + DelegateAction, } /// Both, outgoing and incoming tcp connections to peers, will be rejected if `peer's` @@ -259,6 +261,8 @@ impl ProtocolFeature { ProtocolFeature::FixContractLoadingCost => 129, #[cfg(feature = "protocol_feature_account_id_in_function_call_permission")] ProtocolFeature::AccountIdInFunctionCallPermission => 130, + #[cfg(feature = "protocol_feature_delegate_action")] + ProtocolFeature::DelegateAction => 131, } } } diff --git a/nearcore/Cargo.toml b/nearcore/Cargo.toml index 2af42703916..138e1ca28ff 100644 --- a/nearcore/Cargo.toml +++ b/nearcore/Cargo.toml @@ -111,6 +111,11 @@ protocol_feature_fix_contract_loading_cost = [ "near-vm-runner/protocol_feature_fix_contract_loading_cost", ] protocol_feature_flat_state = ["near-store/protocol_feature_flat_state", "near-chain/protocol_feature_flat_state", "node-runtime/protocol_feature_flat_state"] +protocol_feature_delegate_action = [ + "node-runtime/protocol_feature_delegate_action", + "near-primitives/protocol_feature_delegate_action", +] + nightly = [ "nightly_protocol", diff --git a/neard/Cargo.toml b/neard/Cargo.toml index 1466fed81ad..bdddaa8a229 100644 --- a/neard/Cargo.toml +++ b/neard/Cargo.toml @@ -60,6 +60,7 @@ rosetta_rpc = ["nearcore/rosetta_rpc"] json_rpc = ["nearcore/json_rpc"] protocol_feature_fix_staking_threshold = ["nearcore/protocol_feature_fix_staking_threshold"] protocol_feature_flat_state = ["nearcore/protocol_feature_flat_state"] +protocol_feature_delegate_action = ["nearcore/protocol_feature_delegate_action"] nightly = [ "nightly_protocol", diff --git a/runtime/runtime/Cargo.toml b/runtime/runtime/Cargo.toml index c0319b5d62a..43f1d8edfba 100644 --- a/runtime/runtime/Cargo.toml +++ b/runtime/runtime/Cargo.toml @@ -38,6 +38,7 @@ default = [] dump_errors_schema = ["near-vm-errors/dump_errors_schema"] protocol_feature_flat_state = ["near-store/protocol_feature_flat_state"] no_cpu_compatibility_checks = ["near-vm-runner/no_cpu_compatibility_checks"] +protocol_feature_delegate_action = [] no_cache = [ "near-vm-runner/no_cache", From f34acda4ad77daaf7cd8cb05bb8f765e76b3e4e0 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 17 Nov 2022 17:43:19 +0200 Subject: [PATCH 18/30] Add "protocol_feature_delegate_action" to "nightly" features --- nearcore/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/nearcore/Cargo.toml b/nearcore/Cargo.toml index 138e1ca28ff..52f7c46a186 100644 --- a/nearcore/Cargo.toml +++ b/nearcore/Cargo.toml @@ -125,6 +125,7 @@ nightly = [ "near-store/nightly", "protocol_feature_fix_staking_threshold", "protocol_feature_fix_contract_loading_cost", + "protocol_feature_delegate_action", ] nightly_protocol = [ "near-primitives/nightly_protocol", From 704c524dcb1b2c9d98fff33a0de907091232242b Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 17 Nov 2022 17:43:52 +0200 Subject: [PATCH 19/30] Create test_delegate_action_deserialization for stable build This test fails if "protocol_feature_delegate_action" is disabled. Therefore another version of this test has been added which works on stable build. --- core/primitives/src/transaction.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index e2a7841132a..20c3bf0a0d3 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -667,6 +667,7 @@ mod tests { } #[test] + #[cfg(feature = "protocol_feature_delegate_action")] fn test_delegate_action_deserialization() { // Expected an error. Buffer is empty assert_eq!( @@ -695,4 +696,18 @@ mod tests { delegate_action ); } + + #[test] + #[cfg(not(feature = "protocol_feature_delegate_action"))] + fn test_delegate_action_deserialization() { + let delegate_action = + create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]); + let serialized_delegate_action = delegate_action.try_to_vec().expect("Expect ok"); + + // Valid action + assert_eq!( + Action::try_from_slice(&serialized_delegate_action).map_err(|e| e.kind()), + Err(ErrorKind::InvalidInput) + ); + } } From 88973ce769f6c8168983ace6e5ef4153b4f1d291 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 17 Nov 2022 17:46:59 +0200 Subject: [PATCH 20/30] Applied cargo fmt --- core/primitives/src/transaction.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 20c3bf0a0d3..c0eb6b94a9f 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -282,10 +282,7 @@ pub struct SignedDelegateAction { #[cfg(not(feature = "protocol_feature_delegate_action"))] impl borsh::de::BorshDeserialize for SignedDelegateAction { fn deserialize(_buf: &mut &[u8]) -> ::core::result::Result { - return Err(Error::new( - ErrorKind::InvalidInput, - "Delegate action isn't supported", - )); + return Err(Error::new(ErrorKind::InvalidInput, "Delegate action isn't supported")); } } @@ -701,7 +698,7 @@ mod tests { #[cfg(not(feature = "protocol_feature_delegate_action"))] fn test_delegate_action_deserialization() { let delegate_action = - create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]); + create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]); let serialized_delegate_action = delegate_action.try_to_vec().expect("Expect ok"); // Valid action From 335537ec73dbbbfb459b2c92438b4dde3af1522e Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 17 Nov 2022 17:53:19 +0200 Subject: [PATCH 21/30] Updated snapshots for tests::test_json_unchanged test Cause: runtime::config_store::tests::test_json_unchanged fails The following command created the snapshots: `cargo insta test --accept -p near-primitives -- test_json_unchanged` --- ...ear_primitives__runtime__config_store__tests__0.json.snap | 5 +++++ ...ar_primitives__runtime__config_store__tests__42.json.snap | 5 +++++ ...ar_primitives__runtime__config_store__tests__48.json.snap | 5 +++++ ...ar_primitives__runtime__config_store__tests__49.json.snap | 5 +++++ ...ar_primitives__runtime__config_store__tests__50.json.snap | 5 +++++ ...ar_primitives__runtime__config_store__tests__52.json.snap | 5 +++++ ...ar_primitives__runtime__config_store__tests__53.json.snap | 5 +++++ ...itives__runtime__config_store__tests__testnet_0.json.snap | 5 +++++ ...tives__runtime__config_store__tests__testnet_42.json.snap | 5 +++++ ...tives__runtime__config_store__tests__testnet_48.json.snap | 5 +++++ ...tives__runtime__config_store__tests__testnet_49.json.snap | 5 +++++ ...tives__runtime__config_store__tests__testnet_50.json.snap | 5 +++++ ...tives__runtime__config_store__tests__testnet_52.json.snap | 5 +++++ ...tives__runtime__config_store__tests__testnet_53.json.snap | 5 +++++ 14 files changed, 70 insertions(+) diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__0.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__0.json.snap index 27967578168..8b659f0c87d 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__0.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__0.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__42.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__42.json.snap index cdd3a59e144..d6e5a4f0633 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__42.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__42.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__48.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__48.json.snap index 9cfea46f7ce..1d73753cc92 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__48.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__48.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__49.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__49.json.snap index c3c4c0abf98..b74495c8892 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__49.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__49.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__50.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__50.json.snap index 07bb9527bde..1a3ab756d54 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__50.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__50.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__52.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__52.json.snap index 080e25788d1..843de731144 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__52.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__52.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__53.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__53.json.snap index 541b8e67187..685e6578c1f 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__53.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__53.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_0.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_0.json.snap index 27967578168..8b659f0c87d 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_0.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_0.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_42.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_42.json.snap index cdd3a59e144..d6e5a4f0633 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_42.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_42.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_48.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_48.json.snap index 9cfea46f7ce..1d73753cc92 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_48.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_48.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_49.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_49.json.snap index c3c4c0abf98..b74495c8892 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_49.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_49.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_50.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_50.json.snap index 07bb9527bde..1a3ab756d54 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_50.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_50.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_52.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_52.json.snap index 080e25788d1..843de731144 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_52.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_52.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_53.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_53.json.snap index 541b8e67187..685e6578c1f 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_53.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_53.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { From 010d718a75e5095e6983b4e2786a0df2401c1c78 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 17 Nov 2022 19:11:51 +0200 Subject: [PATCH 22/30] Update DelegateAction feature version number - Update PROTOCOL_VERSION - Update ProtocolFeature::DelegateAction version number --- core/primitives/src/version.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/primitives/src/version.rs b/core/primitives/src/version.rs index bc1130baaf6..bda6dcd4a2c 100644 --- a/core/primitives/src/version.rs +++ b/core/primitives/src/version.rs @@ -147,12 +147,12 @@ pub enum ProtocolFeature { FixContractLoadingCost, #[cfg(feature = "protocol_feature_ed25519_verify")] Ed25519Verify, - #[cfg(feature = "protocol_feature_delegate_action")] - DelegateAction, #[cfg(feature = "protocol_feature_reject_blocks_with_outdated_protocol_version")] RejectBlocksWithOutdatedProtocolVersions, #[cfg(feature = "shardnet")] ShardnetShardLayoutUpgrade, + #[cfg(feature = "protocol_feature_delegate_action")] + DelegateAction, } /// Both, outgoing and incoming tcp connections to peers, will be rejected if `peer's` @@ -168,7 +168,7 @@ const STABLE_PROTOCOL_VERSION: ProtocolVersion = 57; /// Largest protocol version supported by the current binary. pub const PROTOCOL_VERSION: ProtocolVersion = if cfg!(feature = "nightly_protocol") { // On nightly, pick big enough version to support all features. - 132 + 133 } else if cfg!(feature = "shardnet") { 102 } else { @@ -246,8 +246,6 @@ impl ProtocolFeature { ProtocolFeature::FixContractLoadingCost => 129, #[cfg(feature = "protocol_feature_ed25519_verify")] ProtocolFeature::Ed25519Verify => 131, - #[cfg(feature = "protocol_feature_delegate_action")] - ProtocolFeature::DelegateAction => 132, #[cfg(feature = "protocol_feature_reject_blocks_with_outdated_protocol_version")] ProtocolFeature::RejectBlocksWithOutdatedProtocolVersions => { if cfg!(feature = "shardnet") { @@ -258,6 +256,8 @@ impl ProtocolFeature { } #[cfg(feature = "shardnet")] ProtocolFeature::ShardnetShardLayoutUpgrade => 102, + #[cfg(feature = "protocol_feature_delegate_action")] + ProtocolFeature::DelegateAction => 133, } } } From 19354994877564ec3d8b8034bb4446a78b729cbb Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 17 Nov 2022 19:17:53 +0200 Subject: [PATCH 23/30] Apply cargo fmt --- core/primitives/src/views.rs | 8 ++++---- runtime/runtime/src/actions.rs | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index f7e1401b8e0..9ff725a399a 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -36,10 +36,10 @@ use crate::sharding::{ ShardChunkHeaderV3, }; use crate::transaction::{ - Action, AddKeyAction, CreateAccountAction, DelegateAction, DeleteAccountAction, DeleteKeyAction, - DeployContractAction, ExecutionMetadata, ExecutionOutcome, ExecutionOutcomeWithIdAndProof, - ExecutionStatus, FunctionCallAction, PartialExecutionOutcome, PartialExecutionStatus, - SignedDelegateAction,SignedTransaction, StakeAction, TransferAction, + Action, AddKeyAction, CreateAccountAction, DelegateAction, DeleteAccountAction, + DeleteKeyAction, DeployContractAction, ExecutionMetadata, ExecutionOutcome, + ExecutionOutcomeWithIdAndProof, ExecutionStatus, FunctionCallAction, PartialExecutionOutcome, + PartialExecutionStatus, SignedDelegateAction, SignedTransaction, StakeAction, TransferAction, }; use crate::types::{ AccountId, AccountWithPublicKey, Balance, BlockHeight, CompiledContractCache, EpochHeight, diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 766c5a75a78..e4debbf1d32 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -1,4 +1,7 @@ -use crate::config::{safe_add_gas, RuntimeConfig, total_prepaid_exec_fees, total_prepaid_gas, total_prepaid_send_fees}; +use crate::config::{ + safe_add_gas, total_prepaid_exec_fees, total_prepaid_gas, total_prepaid_send_fees, + RuntimeConfig, +}; use crate::ext::{ExternalError, RuntimeExt}; use crate::{metrics, ActionResult, ApplyState}; use borsh::{BorshDeserialize, BorshSerialize}; @@ -7,7 +10,7 @@ use near_primitives::account::{AccessKey, AccessKeyPermission, Account}; use near_primitives::checked_feature; use near_primitives::config::ViewConfig; use near_primitives::contract::ContractCode; -use near_primitives::errors::{ActionError, ActionErrorKind, RuntimeError, InvalidAccessKeyError}; +use near_primitives::errors::{ActionError, ActionErrorKind, InvalidAccessKeyError, RuntimeError}; use near_primitives::hash::CryptoHash; use near_primitives::receipt::{ActionReceipt, Receipt, ReceiptEnum}; use near_primitives::runtime::config::AccountCreationConfig; From 79915fda8969ab468db4fb44e37eb9533889b652 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Thu, 17 Nov 2022 23:18:13 +0200 Subject: [PATCH 24/30] Update snapshots and error schema for tests --- chain/chain/src/tests/simple_chain.rs | 4 +- chain/jsonrpc/res/rpc_errors_schema.json | 56 ++++++++++++++++++- ...runtime__config_store__tests__57.json.snap | 5 ++ ..._config_store__tests__testnet_57.json.snap | 5 ++ 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/chain/chain/src/tests/simple_chain.rs b/chain/chain/src/tests/simple_chain.rs index b2008c7b301..4817252b1a3 100644 --- a/chain/chain/src/tests/simple_chain.rs +++ b/chain/chain/src/tests/simple_chain.rs @@ -42,7 +42,7 @@ fn build_chain() { // cargo insta test --accept -p near-chain --features nightly -- tests::simple_chain::build_chain let hash = chain.head().unwrap().last_block_hash; if cfg!(feature = "nightly") { - insta::assert_display_snapshot!(hash, @"HTpETHnBkxcX1h3eD87uC5YP5nV66E6UYPrJGnQHuRqt"); + insta::assert_display_snapshot!(hash, @"96KiRJdbMN8A9cFPXarZdaRQ8U2HvYcrGTGC8a4EgFzM"); } else { insta::assert_display_snapshot!(hash, @"2iGtRFjF6BcqPF6tDcfLLojRaNax2PKDLxRqRc3RxRn7"); } @@ -72,7 +72,7 @@ fn build_chain() { let hash = chain.head().unwrap().last_block_hash; if cfg!(feature = "nightly") { - insta::assert_display_snapshot!(hash, @"HyDYbjs5tgeEDf1N1XB4m312VdCeKjHqeGQ7dc7Lqwv8"); + insta::assert_display_snapshot!(hash, @"4eW4jvyu1Ek6WmY3EuUoFFkrascC7svRww5UcZbNMkUf"); } else { insta::assert_display_snapshot!(hash, @"7BkghFM7ZA8piYHAWYu4vTY6vE1pkTwy14bqQnS138qE"); } diff --git a/chain/jsonrpc/res/rpc_errors_schema.json b/chain/jsonrpc/res/rpc_errors_schema.json index 4401321ff69..4606910cb67 100644 --- a/chain/jsonrpc/res/rpc_errors_schema.json +++ b/chain/jsonrpc/res/rpc_errors_schema.json @@ -460,7 +460,13 @@ "FunctionCallError", "NewReceiptValidationError", "OnlyImplicitAccountCreationAllowed", - "DeleteAccountWithLargeState" + "DeleteAccountWithLargeState", + "DelegateActionInvalidSignature", + "DelegateActionSenderDoesNotMatchTxReceiver", + "DelegateActionExpired", + "DelegateActionAccessKeyError", + "DelegateActionInvalidNonce", + "DelegateActionNonceTooLarge" ], "props": { "index": "" @@ -480,7 +486,9 @@ "FunctionCallMethodNameLengthExceeded", "FunctionCallArgumentsLengthExceeded", "UnsuitableStakingKey", - "FunctionCallZeroAttachedGas" + "FunctionCallZeroAttachedGas", + "DelegateActionCantContainNestedOne", + "DelegateActionMustBeOnlyOne" ], "props": {} }, @@ -556,6 +564,50 @@ "registrar_account_id": "" } }, + "DelegateActionCantContainNestedOne": { + "name": "DelegateActionCantContainNestedOne", + "subtypes": [], + "props": {} + }, + "DelegateActionExpired": { + "name": "DelegateActionExpired", + "subtypes": [], + "props": {} + }, + "DelegateActionInvalidNonce": { + "name": "DelegateActionInvalidNonce", + "subtypes": [], + "props": { + "ak_nonce": "", + "delegate_nonce": "" + } + }, + "DelegateActionInvalidSignature": { + "name": "DelegateActionInvalidSignature", + "subtypes": [], + "props": {} + }, + "DelegateActionMustBeOnlyOne": { + "name": "DelegateActionMustBeOnlyOne", + "subtypes": [], + "props": {} + }, + "DelegateActionNonceTooLarge": { + "name": "DelegateActionNonceTooLarge", + "subtypes": [], + "props": { + "delegate_nonce": "", + "upper_bound": "" + } + }, + "DelegateActionSenderDoesNotMatchTxReceiver": { + "name": "DelegateActionSenderDoesNotMatchTxReceiver", + "subtypes": [], + "props": { + "receiver_id": "", + "sender_id": "" + } + }, "DeleteAccountStaking": { "name": "DeleteAccountStaking", "subtypes": [], diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__57.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__57.json.snap index 529a8593a87..c7c45874652 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__57.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__57.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { diff --git a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_57.json.snap b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_57.json.snap index 529a8593a87..c7c45874652 100644 --- a/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_57.json.snap +++ b/core/primitives/src/runtime/snapshots/near_primitives__runtime__config_store__tests__testnet_57.json.snap @@ -84,6 +84,11 @@ expression: store.get_config(*version) "send_sir": 147489000000, "send_not_sir": 147489000000, "execution": 147489000000 + }, + "delegate_cost": { + "send_sir": 2319861500000, + "send_not_sir": 2319861500000, + "execution": 2319861500000 } }, "storage_usage_config": { From 155054f086a86299237aa47eb7c96fe2cbf39f6f Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Fri, 18 Nov 2022 14:36:16 +0200 Subject: [PATCH 25/30] Use `safe_add_gas` in `apply_delegate_action` function --- runtime/runtime/src/actions.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index e4debbf1d32..416484a2199 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -676,9 +676,9 @@ pub(crate) fn apply_delegate_action( apply_state.current_protocol_version, )?; let required_gas = receipt_required_gas(apply_state, &new_receipt)?; - result.gas_used += required_gas; - result.gas_used += prepaid_send_fees; - result.gas_burnt += prepaid_send_fees; + result.gas_used = safe_add_gas(result.gas_used, required_gas)?; + result.gas_used = safe_add_gas(result.gas_used, prepaid_send_fees)?; + result.gas_burnt = safe_add_gas(result.gas_burnt, prepaid_send_fees)?; result.new_receipts.push(new_receipt); Ok(()) From 81f4eb772b043bccdf33ed1995c5ee7276c20958 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Fri, 25 Nov 2022 03:35:43 +0200 Subject: [PATCH 26/30] Add comments which explain what's going on --- core/primitives/res/runtime_configs/parameters.txt | 1 + .../res/runtime_configs/parameters_testnet.txt | 1 + runtime/runtime/src/actions.rs | 9 +++++++++ runtime/runtime/src/config.rs | 6 ++++++ 4 files changed, 17 insertions(+) diff --git a/core/primitives/res/runtime_configs/parameters.txt b/core/primitives/res/runtime_configs/parameters.txt index 9815660b8d8..f3de1f5c943 100644 --- a/core/primitives/res/runtime_configs/parameters.txt +++ b/core/primitives/res/runtime_configs/parameters.txt @@ -66,6 +66,7 @@ action_add_function_call_key_per_byte_execution: 1_925_331 action_delete_key_send_sir: 94_946_625_000 action_delete_key_send_not_sir: 94_946_625_000 action_delete_key_execution: 94_946_625_000 +# TODO: place-holder values, needs estimation, tracked in #8114 action_delegate_send_sir: 2_319_861_500_000 action_delegate_send_not_sir: 2_319_861_500_000 action_delegate_execution: 2_319_861_500_000 diff --git a/core/primitives/res/runtime_configs/parameters_testnet.txt b/core/primitives/res/runtime_configs/parameters_testnet.txt index 9508123bfee..13aaaf90d50 100644 --- a/core/primitives/res/runtime_configs/parameters_testnet.txt +++ b/core/primitives/res/runtime_configs/parameters_testnet.txt @@ -62,6 +62,7 @@ action_add_function_call_key_per_byte_execution: 1_925_331 action_delete_key_send_sir: 94_946_625_000 action_delete_key_send_not_sir: 94_946_625_000 action_delete_key_execution: 94_946_625_000 +# TODO: place-holder values, needs estimation, tracked in #8114 action_delegate_send_sir: 2_319_861_500_000 action_delegate_send_not_sir: 2_319_861_500_000 action_delegate_execution: 2_319_861_500_000 diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 416484a2199..117c4b107bc 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -670,13 +670,22 @@ pub(crate) fn apply_delegate_action( action_receipt.gas_price, ); + // Note, Relayer prepaid all fees and all things required by actions: attached deposits and attached gas. + // If something goes wrong, deposit is refunded to the predecessor, this is sender_id/Sender in DelegateAction. + // Gas is refunded to the signer, this is Relayer. + // Some contracts refund the deposit. Usually they refund the deposit to the predecessor and this is sender_id/Sender from DelegateAction. + // Therefore Relayer should verify DelegateAction before submitting it because it spends the attached deposit. + let prepaid_send_fees = total_prepaid_send_fees( &apply_state.config.transaction_costs, &action_receipt.actions, apply_state.current_protocol_version, )?; let required_gas = receipt_required_gas(apply_state, &new_receipt)?; + // This gas will be burnt by the receiver of the created receipt, result.gas_used = safe_add_gas(result.gas_used, required_gas)?; + // This gas was prepaid on Relayer shard. Need to burn it because the receipt is goint to be sent. + // gas_used is incremented because otherwise the gas will be refunded. Refund function checks only gas_used. result.gas_used = safe_add_gas(result.gas_used, prepaid_send_fees)?; result.gas_burnt = safe_add_gas(result.gas_burnt, prepaid_send_fees)?; result.new_receipts.push(new_receipt); diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index bd9023ae676..ba42bee4f24 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -140,6 +140,10 @@ pub fn total_send_fees( } /// Total sum of gas that needs to be burnt to send the inner actions of DelegateAction +/// +/// This is only relevant for DelegateAction, where the send fees of the inner actions +/// need to be prepaid. All other actions burn send fees directly, so calling this function +/// with other actions will return 0. pub fn total_prepaid_send_fees( config: &RuntimeFeesConfig, actions: &[Action], @@ -318,6 +322,8 @@ pub fn total_deposit(actions: &[Action]) -> Result Date: Thu, 12 Jan 2023 15:49:13 +0200 Subject: [PATCH 27/30] Rename protocol_feature_delegate_action with protocol_feature_nep366_delegate_action --- core/primitives/Cargo.toml | 4 ++-- core/primitives/src/transaction.rs | 8 ++++---- core/primitives/src/version.rs | 4 ++-- nearcore/Cargo.toml | 8 ++++---- neard/Cargo.toml | 2 +- runtime/runtime/Cargo.toml | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index bdb29cd0842..2639902f447 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -48,14 +48,14 @@ protocol_feature_reject_blocks_with_outdated_protocol_version = [] protocol_feature_ed25519_verify = [ "near-primitives-core/protocol_feature_ed25519_verify" ] -protocol_feature_delegate_action = [] +protocol_feature_nep366_delegate_action = [] nightly = [ "nightly_protocol", "protocol_feature_fix_staking_threshold", "protocol_feature_fix_contract_loading_cost", "protocol_feature_reject_blocks_with_outdated_protocol_version", "protocol_feature_ed25519_verify", - "protocol_feature_delegate_action", + "protocol_feature_nep366_delegate_action", ] nightly_protocol = [] diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 24fe8e0a64d..1015776cc80 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -259,14 +259,14 @@ pub struct DelegateAction { /// Public key that is used to sign this delegated action. pub public_key: PublicKey, } -#[cfg_attr(feature = "protocol_feature_delegate_action", derive(BorshDeserialize))] +#[cfg_attr(feature = "protocol_feature_nep366_delegate_action", derive(BorshDeserialize))] #[derive(BorshSerialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] pub struct SignedDelegateAction { pub delegate_action: DelegateAction, pub signature: Signature, } -#[cfg(not(feature = "protocol_feature_delegate_action"))] +#[cfg(not(feature = "protocol_feature_nep366_delegate_action"))] impl borsh::de::BorshDeserialize for SignedDelegateAction { fn deserialize(_buf: &mut &[u8]) -> ::core::result::Result { return Err(Error::new(ErrorKind::InvalidInput, "Delegate action isn't supported")); @@ -655,7 +655,7 @@ mod tests { } #[test] - #[cfg(feature = "protocol_feature_delegate_action")] + #[cfg(feature = "protocol_feature_nep366_delegate_action")] fn test_delegate_action_deserialization() { // Expected an error. Buffer is empty assert_eq!( @@ -686,7 +686,7 @@ mod tests { } #[test] - #[cfg(not(feature = "protocol_feature_delegate_action"))] + #[cfg(not(feature = "protocol_feature_nep366_delegate_action"))] fn test_delegate_action_deserialization() { let delegate_action = create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]); diff --git a/core/primitives/src/version.rs b/core/primitives/src/version.rs index bda6dcd4a2c..fd39a6095c9 100644 --- a/core/primitives/src/version.rs +++ b/core/primitives/src/version.rs @@ -151,7 +151,7 @@ pub enum ProtocolFeature { RejectBlocksWithOutdatedProtocolVersions, #[cfg(feature = "shardnet")] ShardnetShardLayoutUpgrade, - #[cfg(feature = "protocol_feature_delegate_action")] + #[cfg(feature = "protocol_feature_nep366_delegate_action")] DelegateAction, } @@ -256,7 +256,7 @@ impl ProtocolFeature { } #[cfg(feature = "shardnet")] ProtocolFeature::ShardnetShardLayoutUpgrade => 102, - #[cfg(feature = "protocol_feature_delegate_action")] + #[cfg(feature = "protocol_feature_nep366_delegate_action")] ProtocolFeature::DelegateAction => 133, } } diff --git a/nearcore/Cargo.toml b/nearcore/Cargo.toml index d6e3d2b71a3..d3937de67ed 100644 --- a/nearcore/Cargo.toml +++ b/nearcore/Cargo.toml @@ -109,9 +109,9 @@ protocol_feature_fix_contract_loading_cost = [ "near-vm-runner/protocol_feature_fix_contract_loading_cost", ] protocol_feature_flat_state = ["near-store/protocol_feature_flat_state", "near-chain/protocol_feature_flat_state", "node-runtime/protocol_feature_flat_state"] -protocol_feature_delegate_action = [ - "node-runtime/protocol_feature_delegate_action", - "near-primitives/protocol_feature_delegate_action", +protocol_feature_nep366_delegate_action = [ + "node-runtime/protocol_feature_nep366_delegate_action", + "near-primitives/protocol_feature_nep366_delegate_action", ] @@ -123,7 +123,7 @@ nightly = [ "near-store/nightly", "protocol_feature_fix_staking_threshold", "protocol_feature_fix_contract_loading_cost", - "protocol_feature_delegate_action", + "protocol_feature_nep366_delegate_action", ] nightly_protocol = [ "near-primitives/nightly_protocol", diff --git a/neard/Cargo.toml b/neard/Cargo.toml index 308f0c69b7d..d60134d652a 100644 --- a/neard/Cargo.toml +++ b/neard/Cargo.toml @@ -62,7 +62,7 @@ json_rpc = ["nearcore/json_rpc"] protocol_feature_fix_staking_threshold = ["nearcore/protocol_feature_fix_staking_threshold"] protocol_feature_flat_state = ["nearcore/protocol_feature_flat_state"] cold_store = ["nearcore/cold_store", "near-store/cold_store", "near-cold-store-tool/cold_store"] -protocol_feature_delegate_action = ["nearcore/protocol_feature_delegate_action"] +protocol_feature_nep366_delegate_action = ["nearcore/protocol_feature_nep366_delegate_action"] nightly = [ "nightly_protocol", diff --git a/runtime/runtime/Cargo.toml b/runtime/runtime/Cargo.toml index d9fc90303cf..bca2a4b78e6 100644 --- a/runtime/runtime/Cargo.toml +++ b/runtime/runtime/Cargo.toml @@ -36,7 +36,7 @@ default = [] dump_errors_schema = ["near-vm-errors/dump_errors_schema"] protocol_feature_flat_state = ["near-store/protocol_feature_flat_state", "near-vm-logic/protocol_feature_flat_state"] no_cpu_compatibility_checks = ["near-vm-runner/no_cpu_compatibility_checks"] -protocol_feature_delegate_action = [] +protocol_feature_nep366_delegate_action = [] no_cache = [ "near-vm-runner/no_cache", From e9e1f8ff5f7123fc0f6f62d948c06aadfb58b789 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Fri, 13 Jan 2023 00:43:13 +0200 Subject: [PATCH 28/30] Add comments and remove `new_delegate_actions` --- core/primitives/src/errors.rs | 4 ++-- core/primitives/src/receipt.rs | 24 ------------------- core/primitives/src/transaction.rs | 6 ++++- runtime/runtime/src/actions.rs | 38 +++++++++++++++++++++--------- runtime/runtime/src/config.rs | 3 ++- runtime/runtime/src/verifier.rs | 1 + 6 files changed, 37 insertions(+), 39 deletions(-) diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 27230b7c84e..d5a98b8cb3f 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -198,7 +198,7 @@ pub enum ActionsValidationError { UnsuitableStakingKey { public_key: PublicKey }, /// The attached amount of gas in a FunctionCall action has to be a positive number. FunctionCallZeroAttachedGas, - /// DelegateAction actions contain another DelegateAction + /// DelegateAction actions contain another DelegateAction. This is not allowed. DelegateActionCantContainNestedOne, /// There should be the only one DelegateAction DelegateActionMustBeOnlyOne, @@ -732,7 +732,7 @@ impl Display for ActionErrorKind { ActionErrorKind::OnlyImplicitAccountCreationAllowed { account_id } => write!(f, "CreateAccount action is called on hex-characters account of length 64 {}", account_id), ActionErrorKind::DeleteAccountWithLargeState { account_id } => write!(f, "The state of account {} is too large and therefore cannot be deleted", account_id), ActionErrorKind::DelegateActionInvalidSignature => write!(f, "DelegateAction is not signed with the given public key"), - ActionErrorKind::DelegateActionSenderDoesNotMatchTxReceiver { sender_id, receiver_id } => write!(f, "Transaction reciever {} doesn't match DelegateAction sender {}", sender_id, receiver_id), + ActionErrorKind::DelegateActionSenderDoesNotMatchTxReceiver { sender_id, receiver_id } => write!(f, "Transaction receiver {} doesn't match DelegateAction sender {}", receiver_id, sender_id), ActionErrorKind::DelegateActionExpired => write!(f, "DelegateAction has expired"), ActionErrorKind::DelegateActionAccessKeyError(access_key_error) => Display::fmt(&access_key_error, f), ActionErrorKind::DelegateActionInvalidNonce { delegate_nonce, ak_nonce } => write!(f, "DelegateAction nonce {} must be larger than nonce of the used access key {}", delegate_nonce, ak_nonce), diff --git a/core/primitives/src/receipt.rs b/core/primitives/src/receipt.rs index 03d1210ac79..e203b12d7a0 100644 --- a/core/primitives/src/receipt.rs +++ b/core/primitives/src/receipt.rs @@ -86,30 +86,6 @@ impl Receipt { }), } } - - pub fn new_delegate_actions( - signer_id: &AccountId, - predecessor_id: &AccountId, - receiver_id: &AccountId, - actions: &Vec, - public_key: &PublicKey, - gas_price: Balance, - ) -> Self { - Receipt { - predecessor_id: predecessor_id.clone(), - receiver_id: receiver_id.clone(), - receipt_id: CryptoHash::default(), - - receipt: ReceiptEnum::Action(ActionReceipt { - signer_id: signer_id.clone(), - signer_public_key: public_key.clone(), - gas_price: gas_price, - output_data_receivers: vec![], - input_data_ids: vec![], - actions: actions.clone(), - }), - } - } } /// Receipt could be either ActionReceipt or DataReceipt diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 1015776cc80..643f762bb91 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -216,6 +216,8 @@ impl From for Action { } } +/// This is Action which mustn't contain DelegateAction. +// This struct is needed to avoid the recursion when Action/DelegateAction is deserialized. #[derive(Serialize, BorshSerialize, Deserialize, PartialEq, Eq, Clone, Debug)] pub struct NonDelegateAction(pub Action); @@ -243,6 +245,7 @@ impl borsh::de::BorshDeserialize for NonDelegateAction { } } +/// This action allows to execute the inner actions behalf of the defined sender. #[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] pub struct DelegateAction { /// Signer of the delegated actions @@ -259,6 +262,7 @@ pub struct DelegateAction { /// Public key that is used to sign this delegated action. pub public_key: PublicKey, } + #[cfg_attr(feature = "protocol_feature_nep366_delegate_action", derive(BorshDeserialize))] #[derive(BorshSerialize, Serialize, Deserialize, PartialEq, Eq, Clone, Debug)] pub struct SignedDelegateAction { @@ -692,7 +696,7 @@ mod tests { create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]); let serialized_delegate_action = delegate_action.try_to_vec().expect("Expect ok"); - // Valid action + // DelegateAction isn't supported assert_eq!( Action::try_from_slice(&serialized_delegate_action).map_err(|e| e.kind()), Err(ErrorKind::InvalidInput) diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 117c4b107bc..a2b00ab0cdf 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -656,19 +656,26 @@ pub(crate) fn apply_delegate_action( validate_delegate_action_key(state_update, apply_state, delegate_action, result)?; if result.result.is_err() { + // Validation failed. Need to return Ok() because this is not a runtime error. + // "result.result" will be return to the User as the action execution result. return Ok(()); } - let actions = delegate_action.get_actions(); - - let new_receipt = Receipt::new_delegate_actions( - &action_receipt.signer_id, - sender_id, - &delegate_action.receiver_id, - &actions, - &action_receipt.signer_public_key, - action_receipt.gas_price, - ); + // Generate a new receipt from DelegateAction. + let new_receipt = Receipt { + predecessor_id: sender_id.clone(), + receiver_id: delegate_action.receiver_id.clone(), + receipt_id: CryptoHash::default(), + + receipt: ReceiptEnum::Action(ActionReceipt { + signer_id: action_receipt.signer_id.clone(), + signer_public_key: action_receipt.signer_public_key.clone(), + gas_price: action_receipt.gas_price, + output_data_receivers: vec![], + input_data_ids: vec![], + actions: delegate_action.get_actions(), + }), + }; // Note, Relayer prepaid all fees and all things required by actions: attached deposits and attached gas. // If something goes wrong, deposit is refunded to the predecessor, this is sender_id/Sender in DelegateAction. @@ -684,7 +691,7 @@ pub(crate) fn apply_delegate_action( let required_gas = receipt_required_gas(apply_state, &new_receipt)?; // This gas will be burnt by the receiver of the created receipt, result.gas_used = safe_add_gas(result.gas_used, required_gas)?; - // This gas was prepaid on Relayer shard. Need to burn it because the receipt is goint to be sent. + // This gas was prepaid on Relayer shard. Need to burn it because the receipt is going to be sent. // gas_used is incremented because otherwise the gas will be refunded. Refund function checks only gas_used. result.gas_used = safe_add_gas(result.gas_used, prepaid_send_fees)?; result.gas_burnt = safe_add_gas(result.gas_burnt, prepaid_send_fees)?; @@ -693,6 +700,7 @@ pub(crate) fn apply_delegate_action( Ok(()) } +/// Returns Gas amount is required to execute Receipt and all actions it contains fn receipt_required_gas(apply_state: &ApplyState, receipt: &Receipt) -> Result { Ok(match &receipt.receipt { ReceiptEnum::Action(action_receipt) => { @@ -716,6 +724,11 @@ fn receipt_required_gas(apply_state: &ApplyState, receipt: &Receipt) -> Result Result Date: Fri, 13 Jan 2023 01:01:43 +0200 Subject: [PATCH 29/30] Add "test_delegate_action_must_be_only_one" test --- runtime/runtime/src/verifier.rs | 52 +++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index 2d78924533b..e546fcc8e9e 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -477,12 +477,12 @@ fn test_truncate_string() { mod tests { use std::sync::Arc; - use near_crypto::{InMemorySigner, KeyType, PublicKey, Signer}; + use near_crypto::{InMemorySigner, KeyType, PublicKey, Signer, Signature}; use near_primitives::account::{AccessKey, Account, FunctionCallPermission}; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::test_utils::account_new; use near_primitives::transaction::{ - CreateAccountAction, DeleteAccountAction, DeleteKeyAction, StakeAction, TransferAction, + CreateAccountAction, DeleteAccountAction, DeleteKeyAction, StakeAction, TransferAction, NonDelegateAction, DelegateAction, }; use near_primitives::types::{AccountId, Balance, MerkleHash, StateChangeCause}; use near_primitives::version::PROTOCOL_VERSION; @@ -1528,4 +1528,52 @@ mod tests { ) .expect("valid action"); } + + #[test] + fn test_delegate_action_must_be_only_one() { + let signed_delegate_action = SignedDelegateAction { + delegate_action: DelegateAction { + sender_id: "bob.test.near".parse().unwrap(), + receiver_id: "token.test.near".parse().unwrap(), + actions: vec![ + NonDelegateAction( + Action::CreateAccount(CreateAccountAction { }) + ) + ], + nonce: 19000001, + max_block_height: 57, + public_key: PublicKey::empty(KeyType::ED25519), + }, + signature: Signature::default() + }; + assert_eq!( + validate_actions( + &VMLimitConfig::test(), + &[ + Action::Delegate(signed_delegate_action.clone()), + Action::Delegate(signed_delegate_action.clone()), + ] + ), + Err(ActionsValidationError::DelegateActionMustBeOnlyOne), + ); + assert_eq!( + validate_actions( + &&VMLimitConfig::test(), + &[ + Action::Delegate(signed_delegate_action.clone()), + ] + ), + Ok(()), + ); + assert_eq!( + validate_actions( + &VMLimitConfig::test(), + &[ + Action::CreateAccount(CreateAccountAction {}), + Action::Delegate(signed_delegate_action.clone()), + ] + ), + Ok(()), + ); + } } From c9eb5059944469031f58880762a6ca7dde2d7181 Mon Sep 17 00:00:00 2001 From: Egor Ulesykiy Date: Fri, 13 Jan 2023 01:25:31 +0200 Subject: [PATCH 30/30] Add a test case for ACTION_DELEGATE_NUMBER --- core/primitives/src/transaction.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 643f762bb91..13503d87b82 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -671,6 +671,9 @@ mod tests { let serialized_non_delegate_action = create_delegate_action(vec![delegate_action]).try_to_vec().expect("Expect ok"); + // Expected Action::Delegate has not been moved in enum Action + assert_eq!(serialized_non_delegate_action[0], ACTION_DELEGATE_NUMBER); + // Expected a nested DelegateAction error assert_eq!( NonDelegateAction::try_from_slice(&serialized_non_delegate_action) @@ -679,7 +682,7 @@ mod tests { ); let delegate_action = - create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]); + create_delegate_action(vec![Action::CreateAccount(CreateAccountAction {})]); let serialized_delegate_action = delegate_action.try_to_vec().expect("Expect ok"); // Valid action @@ -687,6 +690,7 @@ mod tests { Action::try_from_slice(&serialized_delegate_action).expect("Expect ok"), delegate_action ); + } #[test]