Skip to content

Commit

Permalink
Prevent rent-paying account creation (#22292)
Browse files Browse the repository at this point in the history
* Fixup typo

* Add new feature

* Add new TransactionError

* Add framework for checking account state before and after transaction processing

* Fail transactions that leave new rent-paying accounts

* Only check rent-state of writable tx accounts

* Review comments: combine process_result success behavior; log and metrics before feature activation

* Fix tests that assume rent-exempt accounts are okay

* Remove test no longer relevant

* Remove native/sysvar special case

* Move metrics submission to report legacy->legacy rent paying transitions as well
  • Loading branch information
CriesofCarrots authored Jan 11, 2022
1 parent a49ef49 commit 637e366
Show file tree
Hide file tree
Showing 24 changed files with 819 additions and 179 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

3 changes: 2 additions & 1 deletion accountsdb-plugin-postgres/scripts/create_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ Create TYPE "TransactionErrorCode" AS ENUM (
'AddressLookupTableNotFound',
'InvalidAddressLookupTableOwner',
'InvalidAddressLookupTableData',
'InvalidAddressLookupTableIndex'
'InvalidAddressLookupTableIndex',
'InvalidRentPayingAccount'
);

CREATE TYPE "TransactionError" AS (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ pub enum DbTransactionErrorCode {
InvalidAddressLookupTableOwner,
InvalidAddressLookupTableData,
InvalidAddressLookupTableIndex,
InvalidRentPayingAccount,
}

impl From<&TransactionError> for DbTransactionErrorCode {
Expand Down Expand Up @@ -376,6 +377,7 @@ impl From<&TransactionError> for DbTransactionErrorCode {
TransactionError::InvalidAddressLookupTableIndex => {
Self::InvalidAddressLookupTableIndex
}
TransactionError::InvalidRentPayingAccount => Self::InvalidRentPayingAccount,
}
}
}
Expand Down
95 changes: 68 additions & 27 deletions cli/tests/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use {
solana_sdk::{
commitment_config::CommitmentConfig,
hash::Hash,
native_token::sol_to_lamports,
pubkey::Pubkey,
signature::{keypair_from_seed, Keypair, Signer},
system_program,
Expand Down Expand Up @@ -73,10 +74,14 @@ fn full_battery_tests(
&rpc_client,
&config_payer,
&config_payer.signers[0].pubkey(),
2000,
sol_to_lamports(2000.0),
)
.unwrap();
check_recent_balance(2000, &rpc_client, &config_payer.signers[0].pubkey());
check_recent_balance(
sol_to_lamports(2000.0),
&rpc_client,
&config_payer.signers[0].pubkey(),
);

let mut config_nonce = CliConfig::recent_for_tests();
config_nonce.json_rpc_url = json_rpc_url;
Expand Down Expand Up @@ -108,12 +113,16 @@ fn full_battery_tests(
seed,
nonce_authority: optional_authority,
memo: None,
amount: SpendAmount::Some(1000),
amount: SpendAmount::Some(sol_to_lamports(1000.0)),
};

process_command(&config_payer).unwrap();
check_recent_balance(1000, &rpc_client, &config_payer.signers[0].pubkey());
check_recent_balance(1000, &rpc_client, &nonce_account);
check_recent_balance(
sol_to_lamports(1000.0),
&rpc_client,
&config_payer.signers[0].pubkey(),
);
check_recent_balance(sol_to_lamports(1000.0), &rpc_client, &nonce_account);

// Get nonce
config_payer.signers.pop();
Expand Down Expand Up @@ -161,12 +170,16 @@ fn full_battery_tests(
nonce_authority: index,
memo: None,
destination_account_pubkey: payee_pubkey,
lamports: 100,
lamports: sol_to_lamports(100.0),
};
process_command(&config_payer).unwrap();
check_recent_balance(1000, &rpc_client, &config_payer.signers[0].pubkey());
check_recent_balance(900, &rpc_client, &nonce_account);
check_recent_balance(100, &rpc_client, &payee_pubkey);
check_recent_balance(
sol_to_lamports(1000.0),
&rpc_client,
&config_payer.signers[0].pubkey(),
);
check_recent_balance(sol_to_lamports(900.0), &rpc_client, &nonce_account);
check_recent_balance(sol_to_lamports(100.0), &rpc_client, &payee_pubkey);

// Show nonce account
config_payer.command = CliCommand::ShowNonceAccount {
Expand Down Expand Up @@ -208,12 +221,16 @@ fn full_battery_tests(
nonce_authority: 1,
memo: None,
destination_account_pubkey: payee_pubkey,
lamports: 100,
lamports: sol_to_lamports(100.0),
};
process_command(&config_payer).unwrap();
check_recent_balance(1000, &rpc_client, &config_payer.signers[0].pubkey());
check_recent_balance(800, &rpc_client, &nonce_account);
check_recent_balance(200, &rpc_client, &payee_pubkey);
check_recent_balance(
sol_to_lamports(1000.0),
&rpc_client,
&config_payer.signers[0].pubkey(),
);
check_recent_balance(sol_to_lamports(800.0), &rpc_client, &nonce_account);
check_recent_balance(sol_to_lamports(200.0), &rpc_client, &payee_pubkey);
}

#[test]
Expand Down Expand Up @@ -241,18 +258,26 @@ fn test_create_account_with_seed() {
&rpc_client,
&CliConfig::recent_for_tests(),
&offline_nonce_authority_signer.pubkey(),
42,
sol_to_lamports(42.0),
)
.unwrap();
request_and_confirm_airdrop(
&rpc_client,
&CliConfig::recent_for_tests(),
&online_nonce_creator_signer.pubkey(),
4242,
sol_to_lamports(4242.0),
)
.unwrap();
check_recent_balance(42, &rpc_client, &offline_nonce_authority_signer.pubkey());
check_recent_balance(4242, &rpc_client, &online_nonce_creator_signer.pubkey());
check_recent_balance(
sol_to_lamports(42.0),
&rpc_client,
&offline_nonce_authority_signer.pubkey(),
);
check_recent_balance(
sol_to_lamports(4242.0),
&rpc_client,
&online_nonce_creator_signer.pubkey(),
);
check_recent_balance(0, &rpc_client, &to_address);

check_ready(&rpc_client);
Expand All @@ -273,12 +298,20 @@ fn test_create_account_with_seed() {
seed: Some(seed),
nonce_authority: Some(authority_pubkey),
memo: None,
amount: SpendAmount::Some(241),
amount: SpendAmount::Some(sol_to_lamports(241.0)),
};
process_command(&creator_config).unwrap();
check_recent_balance(241, &rpc_client, &nonce_address);
check_recent_balance(42, &rpc_client, &offline_nonce_authority_signer.pubkey());
check_recent_balance(4000, &rpc_client, &online_nonce_creator_signer.pubkey());
check_recent_balance(sol_to_lamports(241.0), &rpc_client, &nonce_address);
check_recent_balance(
sol_to_lamports(42.0),
&rpc_client,
&offline_nonce_authority_signer.pubkey(),
);
check_recent_balance(
sol_to_lamports(4000.999999999),
&rpc_client,
&online_nonce_creator_signer.pubkey(),
);
check_recent_balance(0, &rpc_client, &to_address);

// Fetch nonce hash
Expand All @@ -299,7 +332,7 @@ fn test_create_account_with_seed() {
authority_config.command = CliCommand::ClusterVersion;
process_command(&authority_config).unwrap_err();
authority_config.command = CliCommand::Transfer {
amount: SpendAmount::Some(10),
amount: SpendAmount::Some(sol_to_lamports(10.0)),
to: to_address,
from: 0,
sign_only: true,
Expand All @@ -325,7 +358,7 @@ fn test_create_account_with_seed() {
submit_config.json_rpc_url = test_validator.rpc_url();
submit_config.signers = vec![&authority_presigner];
submit_config.command = CliCommand::Transfer {
amount: SpendAmount::Some(10),
amount: SpendAmount::Some(sol_to_lamports(10.0)),
to: to_address,
from: 0,
sign_only: false,
Expand All @@ -344,8 +377,16 @@ fn test_create_account_with_seed() {
derived_address_program_id: None,
};
process_command(&submit_config).unwrap();
check_recent_balance(241, &rpc_client, &nonce_address);
check_recent_balance(31, &rpc_client, &offline_nonce_authority_signer.pubkey());
check_recent_balance(4000, &rpc_client, &online_nonce_creator_signer.pubkey());
check_recent_balance(10, &rpc_client, &to_address);
check_recent_balance(sol_to_lamports(241.0), &rpc_client, &nonce_address);
check_recent_balance(
sol_to_lamports(31.999999999),
&rpc_client,
&offline_nonce_authority_signer.pubkey(),
);
check_recent_balance(
sol_to_lamports(4000.999999999),
&rpc_client,
&online_nonce_creator_signer.pubkey(),
);
check_recent_balance(sol_to_lamports(10.0), &rpc_client, &to_address);
}
5 changes: 3 additions & 2 deletions cli/tests/request_airdrop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use {
solana_faucet::faucet::run_local_faucet,
solana_sdk::{
commitment_config::CommitmentConfig,
native_token::sol_to_lamports,
signature::{Keypair, Signer},
},
solana_streamer::socket::SocketAddrSpace,
Expand All @@ -22,7 +23,7 @@ fn test_cli_request_airdrop() {
bob_config.json_rpc_url = test_validator.rpc_url();
bob_config.command = CliCommand::Airdrop {
pubkey: None,
lamports: 50,
lamports: sol_to_lamports(50.0),
};
let keypair = Keypair::new();
bob_config.signers = vec![&keypair];
Expand All @@ -36,5 +37,5 @@ fn test_cli_request_airdrop() {
let balance = rpc_client
.get_balance(&bob_config.signers[0].pubkey())
.unwrap();
assert_eq!(balance, 50);
assert_eq!(balance, sol_to_lamports(50.0));
}
6 changes: 3 additions & 3 deletions cli/tests/stake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,7 @@ fn test_offline_nonced_create_stake_account_and_withdraw() {
config_offline.command = CliCommand::WithdrawStake {
stake_account_pubkey: stake_pubkey,
destination_account_pubkey: recipient_pubkey,
amount: SpendAmount::Some(42),
amount: SpendAmount::Some(50_000),
withdraw_authority: 0,
custodian: None,
sign_only: true,
Expand All @@ -1591,7 +1591,7 @@ fn test_offline_nonced_create_stake_account_and_withdraw() {
config.command = CliCommand::WithdrawStake {
stake_account_pubkey: stake_pubkey,
destination_account_pubkey: recipient_pubkey,
amount: SpendAmount::Some(42),
amount: SpendAmount::Some(50_000),
withdraw_authority: 0,
custodian: None,
sign_only: false,
Expand All @@ -1607,7 +1607,7 @@ fn test_offline_nonced_create_stake_account_and_withdraw() {
fee_payer: 0,
};
process_command(&config).unwrap();
check_recent_balance(42, &rpc_client, &recipient_pubkey);
check_recent_balance(50_000, &rpc_client, &recipient_pubkey);

// Fetch nonce hash
let nonce_hash = nonce_utils::get_account_with_commitment(
Expand Down
Loading

0 comments on commit 637e366

Please sign in to comment.