Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: meta transaction account creation scenarios #8568

Merged
merged 5 commits into from
Feb 17, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions core/crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ c2-chacha.workspace = true
curve25519-dalek.workspace = true
derive_more.workspace = true
ed25519-dalek.workspace = true
hex.workspace = true
near-account-id = { path = "../account-id" }
once_cell.workspace = true
primitive-types.workspace = true
Expand Down
14 changes: 14 additions & 0 deletions core/crypto/src/test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use borsh::BorshDeserialize;
use secp256k1::rand::SeedableRng;

use crate::signature::{ED25519PublicKey, ED25519SecretKey, KeyType, PublicKey, SecretKey};
Expand Down Expand Up @@ -33,6 +34,19 @@ impl PublicKey {
_ => unimplemented!(),
}
}

pub fn from_implicit_account(account_id: &AccountId) -> Self {
assert!(account_id.is_implicit());
let mut public_key_data = Vec::with_capacity(33);
public_key_data.push(KeyType::ED25519 as u8);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parts of this code seem to be taken/copied almost verbatim from runtime/runtime/src/actions.rs so this would perhaps be better off living in a common (non-test-utils) code that either tests or code could use.

(If that's not feasible for any reason, the actions.rs code could be adjusted at least to replace the magic 0 constant with the KeyType as is done here, as you're touching the relevant code anyway…)

Copy link
Contributor Author

@jakmeier jakmeier Feb 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we even have another instance of this code copied into near-mirror.

But I don't like editing production code in test-only PRs that are by themself already quite complicated. I'd rather fix this in a separate issue:

#8588

public_key_data.extend(
hex::decode(account_id.as_ref().as_bytes())
.expect("account id was a valid hex of length 64 resulting in 32 bytes"),
);
assert_eq!(public_key_data.len(), 33);
PublicKey::try_from_slice(&public_key_data)
.expect("we should be able to deserialize ED25519 public key")
}
}

impl SecretKey {
Expand Down
21 changes: 19 additions & 2 deletions core/primitives/src/test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::HashMap;
use std::sync::Arc;

use near_crypto::{EmptySigner, InMemorySigner, KeyType, PublicKey, Signature, Signer};
use near_crypto::{EmptySigner, InMemorySigner, KeyType, PublicKey, SecretKey, Signature, Signer};
use near_primitives_core::types::ProtocolVersion;

use crate::account::{AccessKey, AccessKeyPermission, Account};
Expand Down Expand Up @@ -491,9 +491,26 @@ pub fn create_test_signer(account_name: &str) -> InMemoryValidatorSigner {

/// Helper function that creates a new signer for a given account, that uses the account name as seed.
///
/// This also works for predefined implicit accounts, where the signer will use the implicit key.
///
/// Should be used only in tests.
pub fn create_user_test_signer(account_name: &str) -> InMemorySigner {
InMemorySigner::from_seed(account_name.parse().unwrap(), KeyType::ED25519, account_name)
let account_id = account_name.parse().unwrap();
if account_id == implicit_test_account() {
InMemorySigner::from_secret_key(account_id, implicit_test_account_secret())
} else {
InMemorySigner::from_seed(account_id, KeyType::ED25519, account_name)
}
}

/// A fixed implicit account for which tests can know the private key.
pub fn implicit_test_account() -> AccountId {
"061b1dd17603213b00e1a1e53ba060ad427cef4887bd34a5e0ef09010af23b0a".parse().unwrap()
}

/// Private key for the fixed implicit test account.
pub fn implicit_test_account_secret() -> SecretKey {
"ed25519:5roj6k68kvZu3UEJFyXSfjdKGrodgZUfFLZFpzYXWtESNsLWhYrq3JGi4YpqeVKuw1m9R2TEHjfgWT1fjUqB1DNy".parse().unwrap()
}

impl FinalExecutionOutcomeView {
Expand Down
177 changes: 163 additions & 14 deletions integration-tests/src/tests/client/features/delegate_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@ use near_client::test_utils::TestEnv;
use near_crypto::{KeyType, PublicKey};
use near_primitives::account::AccessKey;
use near_primitives::config::ActionCosts;
use near_primitives::errors::{ActionsValidationError, InvalidTxError, TxExecutionError};
use near_primitives::test_utils::create_user_test_signer;
use near_primitives::errors::{
ActionError, ActionErrorKind, ActionsValidationError, InvalidTxError, TxExecutionError,
};
use near_primitives::test_utils::{create_user_test_signer, implicit_test_account};
use near_primitives::transaction::{
Action, AddKeyAction, DeleteAccountAction, DeleteKeyAction, DeployContractAction,
FunctionCallAction, StakeAction, TransferAction,
Action, AddKeyAction, CreateAccountAction, DeleteAccountAction, DeleteKeyAction,
DeployContractAction, FunctionCallAction, StakeAction, TransferAction,
};
use near_primitives::types::{AccountId, Balance};
use near_primitives::version::{ProtocolFeature, ProtocolVersion};
use near_primitives::views::{
AccessKeyPermissionView, FinalExecutionOutcomeView, FinalExecutionStatus,
};
use near_test_contracts::smallest_rs_contract;
use near_test_contracts::{ft_contract, smallest_rs_contract};
use nearcore::config::GenesisExt;
use nearcore::NEAR_BASE;
use testlib::runtime_utils::{
Expand Down Expand Up @@ -117,15 +119,17 @@ fn check_meta_tx_execution(

let sender_before = node_user.view_balance(&sender).unwrap();
let relayer_before = node_user.view_balance(&relayer).unwrap();
let receiver_before = node_user.view_balance(&receiver).unwrap();
let receiver_before = node_user.view_balance(&receiver).unwrap_or(0);
let relayer_nonce_before = node_user
.get_access_key(&relayer, &PublicKey::from_seed(KeyType::ED25519, &relayer))
.unwrap()
.nonce;
let user_nonce_before = node_user
.get_access_key(&sender, &PublicKey::from_seed(KeyType::ED25519, &sender))
.unwrap()
.nonce;
let user_pubk = if sender.is_implicit() {
PublicKey::from_implicit_account(&sender)
} else {
PublicKey::from_seed(KeyType::ED25519, &sender)
};
let user_nonce_before = node_user.get_access_key(&sender, &user_pubk).unwrap().nonce;

let tx_result = node_user
.meta_tx(sender.clone(), receiver.clone(), relayer.clone(), actions.clone())
Expand Down Expand Up @@ -171,7 +175,7 @@ fn check_meta_tx_no_fn_call(
receiver: AccountId,
) -> FinalExecutionOutcomeView {
let fee_helper = fee_helper(node);
let gas_cost = normal_tx_cost + fee_helper.meta_tx_overhead_cost(&actions);
let gas_cost = normal_tx_cost + fee_helper.meta_tx_overhead_cost(&actions, &receiver);

let (tx_result, sender_diff, relayer_diff, receiver_diff) =
check_meta_tx_execution(node, actions, sender, relayer, receiver);
Expand Down Expand Up @@ -203,7 +207,7 @@ fn check_meta_tx_fn_call(
) -> FinalExecutionOutcomeView {
let fee_helper = fee_helper(node);
let num_fn_calls = actions.len();
let meta_tx_overhead_cost = fee_helper.meta_tx_overhead_cost(&actions);
let meta_tx_overhead_cost = fee_helper.meta_tx_overhead_cost(&actions, &receiver);

let (tx_result, sender_diff, relayer_diff, receiver_diff) =
check_meta_tx_execution(node, actions, sender, relayer, receiver);
Expand Down Expand Up @@ -401,8 +405,8 @@ fn meta_tx_delete_account() {
vec![Action::DeleteAccount(DeleteAccountAction { beneficiary_id: relayer.clone() })];

// special case balance check for deleting account
let gas_cost =
fee_helper.prepaid_delete_account_cost() + fee_helper.meta_tx_overhead_cost(&actions);
let gas_cost = fee_helper.prepaid_delete_account_cost()
+ fee_helper.meta_tx_overhead_cost(&actions, &receiver);
let (_tx_result, sender_diff, relayer_diff, receiver_diff) =
check_meta_tx_execution(&node, actions, sender, relayer, receiver.clone());

Expand Down Expand Up @@ -572,3 +576,148 @@ fn assert_ft_balance(
let balance = std::str::from_utf8(&response.result).expect("invalid UTF8");
assert_eq!(format!("\"{expected_balance}\""), balance);
}

/// Test account creation scenarios with meta transactions.
///
/// Named accounts aren't the primary use case for meta transactions but still
/// worth a test case.
#[test]
fn meta_tx_create_named_account() {
let relayer = bob_account();
let sender = alice_account();
let new_account = eve_dot_alice_account();
let node = RuntimeNode::new(&relayer);

let fee_helper = fee_helper(&node);
let amount = NEAR_BASE;

let public_key = PublicKey::from_seed(KeyType::ED25519, &new_account);

// That's the minium to create a (useful) account.
jakmeier marked this conversation as resolved.
Show resolved Hide resolved
let actions = vec![
Action::CreateAccount(CreateAccountAction {}),
Action::Transfer(TransferAction { deposit: amount }),
Action::AddKey(AddKeyAction { public_key, access_key: AccessKey::full_access() }),
];

// Check account doesn't exist, yet
node.view_account(&new_account).expect_err("account already exists");

let tx_cost = fee_helper.create_account_transfer_full_key_cost();
check_meta_tx_no_fn_call(&node, actions, tx_cost, amount, sender, relayer, new_account.clone());

// Check account exists
node.view_account(&new_account).expect("failed looking up account");
}

/// Try creating an implicit account with `CreateAction` which is not allowed in
/// or outside meta transactions and must fail with `OnlyImplicitAccountCreationAllowed`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observation: isn’t this error variant a misnomer? We’re saying that you can only create implicit accounts where the expectation really is that you can only create named accounts…?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point, very confusing.

I think the naming wants to say, "you are trying to create a 64 char length account, this is only allowed in the fancy new implicit account creation flow", where "fancy and new" refers back to 2020 and this NEP that introduced the feature: near/NEPs#71

Before that, named accounts were the only type of account.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is confusing for all of us - can we rename error to smth like OnlyNamedAccountCreationAllowed in separate PR? If this is part of protocol, we can try renaming enum name but leave conversion to string as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I believe it's not a protocol change to rename this, the borsh representation should be fine and I don't think we store errors as strings anywhere it matters. But the RPC consumers might care, we have had unfortunate surprises around that in the past.

For now I filed this: #8598

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I added a TODO in the code comment

#[test]
fn meta_tx_create_implicit_account_fails() {
let relayer = bob_account();
let sender = alice_account();
let new_account: AccountId = implicit_test_account();
let node = RuntimeNode::new(&relayer);

let actions = vec![Action::CreateAccount(CreateAccountAction {})];
let tx_result = node
.user()
.meta_tx(sender.clone(), new_account.clone(), relayer.clone(), actions.clone())
.unwrap();

let account_creation_result = &tx_result.receipts_outcome[1].outcome.status;
assert!(matches!(
account_creation_result,
near_primitives::views::ExecutionStatusView::Failure(TxExecutionError::ActionError(
ActionError { kind: ActionErrorKind::OnlyImplicitAccountCreationAllowed { .. }, .. }
)),
));
}

/// Try creating an implicit account with a meta tx transfer and use the account
/// in the same meta transaction.
///
/// This is expected to fail with `AccountDoesNotExist` as noted in NEP-366.
jakmeier marked this conversation as resolved.
Show resolved Hide resolved
#[test]
fn meta_tx_create_and_use_implicit_account() {
let relayer = bob_account();
let sender = alice_account();
let new_account: AccountId = implicit_test_account();
let node = RuntimeNode::new(&relayer);

// Check account doesn't exist, yet
node.view_account(&new_account).expect_err("account already exists");

let initial_amount = nearcore::NEAR_BASE;
let actions = vec![
Action::Transfer(TransferAction { deposit: initial_amount }),
Action::DeployContract(DeployContractAction { code: ft_contract().to_vec() }),
];

// execute and expect `AccountDoesNotExist`
let tx_result =
node.user().meta_tx(sender.clone(), new_account.clone(), relayer.clone(), actions).unwrap();
let status = &tx_result.receipts_outcome[1].outcome.status;
assert!(matches!(
status,
near_primitives::views::ExecutionStatusView::Failure(TxExecutionError::ActionError(
ActionError { kind: ActionErrorKind::AccountDoesNotExist { account_id }, .. }
)) if *account_id == new_account,
));
}

/// Creating an implicit account with a meta tx transfer and use the account in
/// a second meta transaction.
///
/// Creation through a meta tx should work as normal, it's just that the relayer
/// pays for the storage and the user could delete the account and cash in,
/// hence this workflow is not ideal from all circumstances.
#[test]
fn meta_tx_create_implicit_account() {
let relayer = bob_account();
let sender = alice_account();
let new_account: AccountId = implicit_test_account();
let node = RuntimeNode::new(&relayer);

// Check account doesn't exist, yet
node.view_account(&new_account).expect_err("account already exists");

let fee_helper = fee_helper(&node);
let initial_amount = nearcore::NEAR_BASE;
let actions = vec![Action::Transfer(TransferAction { deposit: initial_amount })];
let tx_cost = fee_helper.create_account_transfer_full_key_cost();
check_meta_tx_no_fn_call(
&node,
actions,
tx_cost,
initial_amount,
sender.clone(),
relayer.clone(),
new_account.clone(),
);

// Check account exists with expected balance
node.view_account(&new_account).expect("failed looking up account");
let balance = node.view_balance(&new_account).expect("failed looking up balance");
assert_eq!(balance, initial_amount);

// Now test we can use this account in a meta transaction that sends back half the tokens to alice.
let transfer_amount = initial_amount / 2;
let actions = vec![Action::Transfer(TransferAction { deposit: transfer_amount })];
let tx_cost = fee_helper.transfer_cost();
check_meta_tx_no_fn_call(
&node,
actions,
tx_cost,
transfer_amount,
new_account.clone(),
relayer,
sender,
)
.assert_success();

// balance of the new account should NOT change, the relayer pays for it!
// (note: relayer balance checks etc are done in the shared checker function)
let balance = node.view_balance(&new_account).expect("failed looking up balance");
assert_eq!(balance, initial_amount);
}
1 change: 1 addition & 0 deletions test-utils/testlib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ near-chain = { path = "../../chain/chain" }
near-crypto = { path = "../../core/crypto" }
near-primitives = { path = "../../core/primitives" }
near-test-contracts = { path = "../../runtime/near-test-contracts" }
node-runtime = { path = "../../runtime/runtime" }

[features]
default = []
Expand Down
Loading