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

fix: disallow target account when creating position for integrity pool #506

Merged
merged 3 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions staking/integration-tests/src/staking/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub fn create_position(
target_with_parameters: TargetWithParameters,
pool_authority: Option<&Keypair>,
amount: frac64,
) {
) -> TransactionResult {
let config_pubkey = get_config_address();
let stake_account_metadata = get_stake_account_metadata_address(stake_account_positions);
let stake_account_custody = get_stake_account_custody_address(stake_account_positions);
Expand Down Expand Up @@ -194,7 +194,7 @@ pub fn create_position(
svm.latest_blockhash(),
);

svm.send_transaction(create_position_tx).unwrap();
svm.send_transaction(create_position_tx)
}

pub fn close_position(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ fn test_merge_delegation_positions() {
TargetWithParameters::Voting,
None,
3,
);
)
.unwrap();

let mut stake_positions_account = fetch_positions_account(&mut svm, &stake_account_positions);
let positions = stake_positions_account.to_dynamic_position_array();
Expand Down Expand Up @@ -179,7 +180,8 @@ fn test_merge_delegation_positions() {
TargetWithParameters::Voting,
None,
5,
);
)
.unwrap();

delegate(
&mut svm,
Expand Down
143 changes: 143 additions & 0 deletions staking/integration-tests/tests/pool_authority.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
use {
anchor_lang::{
system_program,
InstructionData,
ToAccountMetas,
},
integration_tests::{
assert_anchor_program_error,
setup::{
setup,
SetupProps,
SetupResult,
},
staking::{
helper_functions::initialize_new_stake_account,
instructions::{
create_position,
update_pool_authority,
},
pda::{
get_config_address,
get_stake_account_custody_address,
get_stake_account_metadata_address,
get_target_address,
},
},
},
integrity_pool::utils::types::FRAC_64_MULTIPLIER,
solana_sdk::{
instruction::Instruction,
signature::Keypair,
signer::Signer,
transaction::Transaction,
},
staking::error::ErrorCode,
};


#[test]
fn test_pool_authority() {
let SetupResult {
mut svm,
payer,
pyth_token_mint,
publisher_keypair,
pool_data_pubkey: _,
reward_program_authority: _,
maybe_publisher_index: _,
} = setup(SetupProps {
init_config: true,
init_target: true,
init_mint: true,
init_pool_data: true,
init_publishers: true,
});

let stake_account_positions =
initialize_new_stake_account(&mut svm, &payer, &pyth_token_mint, true, true);

let pool_authority = Keypair::new();

update_pool_authority(&mut svm, &payer, pool_authority.pubkey());

create_position(
&mut svm,
&payer,
stake_account_positions,
staking::state::positions::TargetWithParameters::IntegrityPool {
publisher: publisher_keypair.pubkey(),
},
Some(&pool_authority),
50 * FRAC_64_MULTIPLIER,
)
.unwrap();

assert_anchor_program_error!(
create_position(
&mut svm,
&payer,
stake_account_positions,
staking::state::positions::TargetWithParameters::IntegrityPool {
publisher: publisher_keypair.pubkey(),
},
None,
50 * FRAC_64_MULTIPLIER,
),
ErrorCode::InvalidPoolAuthority,
0
);

create_position(
&mut svm,
&payer,
stake_account_positions,
staking::state::positions::TargetWithParameters::Voting,
None,
10 * FRAC_64_MULTIPLIER,
)
.unwrap();

let config_pubkey = get_config_address();
let stake_account_metadata = get_stake_account_metadata_address(stake_account_positions);
let stake_account_custody = get_stake_account_custody_address(stake_account_positions);
let voting_target_account = get_target_address();

let create_position_data = staking::instruction::CreatePosition {
target_with_parameters: staking::state::positions::TargetWithParameters::IntegrityPool {
publisher: publisher_keypair.pubkey(),
},
amount: 10 * FRAC_64_MULTIPLIER,
};

let create_position_accs = staking::accounts::CreatePosition {
config: config_pubkey,
stake_account_metadata,
stake_account_positions,
stake_account_custody,
owner: payer.pubkey(),
target_account: Some(voting_target_account),
pool_authority: Some(pool_authority.pubkey()),
system_program: system_program::ID,
};

let create_position_ix = Instruction::new_with_bytes(
staking::ID,
&create_position_data.data(),
create_position_accs.to_account_metas(None),
);


let create_position_tx = Transaction::new_signed_with_payer(
&[create_position_ix],
Some(&payer.pubkey()),
&[&payer, &pool_authority],
svm.latest_blockhash(),
);

assert_anchor_program_error!(
svm.send_transaction(create_position_tx),
ErrorCode::UnexpectedTargetAccount,
0
);
Copy link
Contributor

Choose a reason for hiding this comment

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

this test is duplicated below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the name was wrong, it is actually testing close position. Fixed.

}
3 changes: 2 additions & 1 deletion staking/integration-tests/tests/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,8 @@ fn test_stability(props: StabilityTestProps) {
TargetWithParameters::Voting {},
None,
*amount,
);
)
.unwrap();
operation_counts.get_mut(&operation.get_name()).unwrap().0 += 1;
}
Operation::CloseGovernancePosition { delegator, amount } => {
Expand Down
9 changes: 6 additions & 3 deletions staking/integration-tests/tests/staking_slash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,17 @@ fn test_staking_slash() {
},
Some(&pool_authority),
50 * FRAC_64_MULTIPLIER,
);
)
.unwrap();
create_position(
&mut svm,
&payer,
stake_account_positions,
staking::state::positions::TargetWithParameters::Voting,
None,
10 * FRAC_64_MULTIPLIER,
);
)
.unwrap();
svm.expire_blockhash();

create_position(
Expand All @@ -98,7 +100,8 @@ fn test_staking_slash() {
staking::state::positions::TargetWithParameters::Voting,
None,
80 * FRAC_64_MULTIPLIER,
);
)
.unwrap();

let stake_account_metadata = get_stake_account_metadata_address(stake_account_positions);
let metadata_account: StakeAccountMetadataV2 =
Expand Down
7 changes: 4 additions & 3 deletions staking/integration-tests/tests/voting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ const MAINNET_ELAPSED_EPOCHS: u64 = 2850;
/// This test has two purposes:
/// 1) to test the voting functionality against the deployed governance program and configuration
/// 2) to test that the new staking account is compatible with stake account positions with the old
/// fixed sized position array and such accounts can be turned into the new version by calling
/// merge_target_positions and nothing breaks
/// fixed sized position array and such accounts can be turned into the new version by calling
/// merge_target_positions and nothing breaks
fn test_voting() {
let SetupResult {
mut svm,
Expand Down Expand Up @@ -135,7 +135,8 @@ fn test_voting() {
TargetWithParameters::Voting,
None,
100,
);
)
.unwrap();

let mut positions_account = fetch_positions_account(&mut svm, &stake_account_positions);
let positions = positions_account.to_dynamic_position_array();
Expand Down
4 changes: 3 additions & 1 deletion staking/programs/staking/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ pub enum ErrorCode {
MissingTargetAccount,
#[msg("The slash ratio should be between 0 and 1")] // 6038
InvalidSlashRatio,
#[msg("Other")] //6039
#[msg("The target account is not allowed for this operation")] // 6039
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[msg("The target account is not allowed for this operation")] // 6039
#[msg("The target account is only expected when dealing with the governance target")] // 6039

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

UnexpectedTargetAccount,
#[msg("Other")] //6040
Other,
}
4 changes: 4 additions & 0 deletions staking/programs/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ pub mod staking {
}

if let TargetWithParameters::IntegrityPool { .. } = target_with_parameters {
require!(
Copy link
Contributor

Choose a reason for hiding this comment

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

need to update close_position as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

maybe_target_account.is_none(),
ErrorCode::UnexpectedTargetAccount
);
require!(
ctx.accounts
.pool_authority
Expand Down
5 changes: 5 additions & 0 deletions staking/target/idl/staking.json
Original file line number Diff line number Diff line change
Expand Up @@ -2594,6 +2594,11 @@
},
{
"code": 6039,
"name": "UnexpectedTargetAccount",
"msg": "The target account is not allowed for this operation"
},
{
"code": 6040,
"name": "Other",
"msg": "Other"
}
Expand Down
5 changes: 5 additions & 0 deletions staking/target/types/staking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2600,6 +2600,11 @@ export type Staking = {
},
{
"code": 6039,
"name": "unexpectedTargetAccount",
"msg": "The target account is not allowed for this operation"
},
{
"code": 6040,
"name": "other",
"msg": "other"
}
Expand Down
Loading