Skip to content

Commit

Permalink
add test for hinted cancel with wrong trader index (#11)
Browse files Browse the repository at this point in the history
  • Loading branch information
brittcyr authored Aug 26, 2024
1 parent 32c236c commit eac896a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
9 changes: 5 additions & 4 deletions programs/manifest/src/program/processor/batch_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,22 +173,23 @@ pub(crate) fn process_batch_update(
&global_trade_accounts_opts,
)?;
}
Some(hinted_index) => {
Some(hinted_cancel_index) => {
// TODO: Verify that it is an order at the index, not a
// ClaimedSeat that a malicious user pretended was a seat.
let order: &RestingOrder = dynamic_account.get_order_by_index(hinted_index);
let order: &RestingOrder =
dynamic_account.get_order_by_index(hinted_cancel_index);
// Simple sanity check on the hint given. Make sure that it
// aligns with block boundaries. We do a check that it is an
// order owned by the payer inside the handler.
assert_with_msg(
trader_index % (BLOCK_SIZE as DataIndex) == 0
&& trader_index == order.get_trader_index(),
ManifestError::WrongIndexHintParams,
&format!("Invalid cancel hint index {}", hinted_index),
&format!("Invalid cancel hint index {}", hinted_cancel_index),
)?;
dynamic_account.cancel_order_by_index(
trader_index,
hinted_index,
hinted_cancel_index,
&global_trade_accounts_opts,
)?;
}
Expand Down
1 change: 0 additions & 1 deletion programs/manifest/src/state/market.rs
Original file line number Diff line number Diff line change
Expand Up @@ -901,7 +901,6 @@ impl<Fixed: DerefOrBorrowMut<MarketFixed>, Dynamic: DerefOrBorrowMut<[u8]>>
}
remove_order_from_tree_and_free(fixed, dynamic, order_index, is_bid)?;

// TODO: Assert that the trader_index is the owner of the order
Ok(())
}
}
Expand Down
21 changes: 18 additions & 3 deletions programs/manifest/tests/cases/cancel_order.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use std::cell::RefMut;

use hypertree::DataIndex;
use manifest::{
program::{batch_update::CancelOrderParams, batch_update_instruction},
state::{OrderType, BLOCK_SIZE},
};
use solana_program_test::{tokio, ProgramTestContext};
use solana_sdk::{instruction::Instruction, signer::Signer, transaction::Transaction};
use solana_sdk::{
instruction::Instruction, signature::Keypair, signer::Signer, transaction::Transaction,
};

use crate::{Side, TestFixture, Token, SOL_UNIT_SIZE, USDC_UNIT_SIZE};

Expand Down Expand Up @@ -47,8 +50,7 @@ async fn cancel_order_fail_other_trader_order_test() -> anyhow::Result<()> {
test_fixture.claim_seat().await?;

// Should succeed. It was funded with infinite lamports.
let second_keypair: solana_sdk::signature::Keypair =
test_fixture.second_keypair.insecure_clone();
let second_keypair: Keypair = test_fixture.second_keypair.insecure_clone();
test_fixture.claim_seat_for_keypair(&second_keypair).await?;
test_fixture
.deposit_for_keypair(Token::SOL, 2 * SOL_UNIT_SIZE, &second_keypair)
Expand All @@ -67,6 +69,19 @@ async fn cancel_order_fail_other_trader_order_test() -> anyhow::Result<()> {

assert!(test_fixture.cancel_order(0).await.is_err());

assert!(test_fixture
.batch_update_for_keypair(
None,
vec![CancelOrderParams::new_with_hint(
0,
Some((BLOCK_SIZE * 2) as DataIndex)
)],
vec![],
&test_fixture.payer_keypair()
)
.await
.is_err());

Ok(())
}

Expand Down

0 comments on commit eac896a

Please sign in to comment.