Skip to content

Commit

Permalink
fix(contracts): ensure caller is canceller & canceler is the offerer (#…
Browse files Browse the repository at this point in the history
…450)

## Description

Add check in `cancel_order` to ensure:
- caller is the canceller
- canceller is the offerer

<!--
Please do not leave this blank.
Describe the changes in this PR. What does it [add/remove/fix/replace]?

For crafting a good description, consider using ChatGPT to help
articulate your changes.
-->

## What type of PR is this? (check all applicable)

- [ ] 🍕 Feature (`feat:`)
- [x] 🐛 Bug Fix (`fix:`)
- [ ] 📝 Documentation Update (`docs:`)
- [ ] 🎨 Style (`style:`)
- [ ] 🧑‍💻 Code Refactor (`refactor:`)
- [ ] 🔥 Performance Improvements (`perf:`)
- [x] ✅ Test (`test:`)
- [ ] 🤖 Build (`build:`)
- [ ] 🔁 CI (`ci:`)
- [ ] 📦 Chore (`chore:`)
- [ ] ⏩ Revert (`revert:`)
- [ ] 🚀 Breaking Changes (`BREAKING CHANGE:`)

## Related Tickets & Documents

<!--
Please use this format to link related issues: Fixes #<issue_number>
More info:
https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->

## Added tests?

- [ ] 👍 yes
- [ ] 🙅 no, because they aren't needed
- [ ] 🙋 no, because I need help

## Added to documentation?

- [ ] 📜 README.md
- [ ] 📓 Documentation
- [ ] 🙅 no documentation needed

## [optional] Are there any post-deployment tasks we need to perform?

<!-- Describe any additional tasks, if any, and provide steps. -->

## [optional] What gif best describes this PR or how it makes you feel?

<!-- Share a fun gif related to your PR! -->

### PR Title and Description Guidelines:

- Ensure your PR title follows semantic versioning standards. This helps
automate releases and changelogs.
- Use types like `feat:`, `fix:`, `chore:`, `BREAKING CHANGE:` etc. in
your PR title.
- Your PR title will be used as a commit message when merging. Make sure
it adheres to [Conventional Commits
standards](https://www.conventionalcommits.org/).

## Closing Issues

<!--
Use keywords to close related issues. This ensures that the associated
issues will automatically close when the PR is merged.

- `Fixes #123` will close issue 123 when the PR is merged.
- `Closes #123` will also close issue 123 when the PR is merged.
- `Resolves #123` will also close issue 123 when the PR is merged.

You can also use multiple keywords in one comment:
- `Fixes #123, Resolves #456`

More info:
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
-->
  • Loading branch information
ptisserand authored Sep 11, 2024
1 parent 28ff5c0 commit 9fcb990
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 0 deletions.
11 changes: 11 additions & 0 deletions contracts/ark_starknet/src/executor.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ mod executor {
};

let vinfo = CancelOrderInfo { cancelInfo: cancelInfo.clone() };
_verify_cancel_order(@self, @vinfo);

let mut vinfo_buf = array![];
Serde::serialize(@vinfo, ref vinfo_buf);
Expand Down Expand Up @@ -546,6 +547,16 @@ mod executor {
}
}

fn _verify_cancel_order(self: @ContractState, vinfo: @CancelOrderInfo) {
let cancel_info = vinfo.cancelInfo;
let caller = starknet::get_caller_address();
let canceller = *(cancel_info.canceller);
assert!(caller == canceller, "Caller is not the canceller");

let order_info = self.orders.read(*cancel_info.order_hash);
assert!(order_info.offerer == canceller, "Canceller is not the offerer");
}

fn _verify_fulfill_order(self: @ContractState, vinfo: @FulfillOrderInfo) {
let fulfill_info = vinfo.fulfillInfo;
let caller = starknet::get_caller_address();
Expand Down
114 changes: 114 additions & 0 deletions contracts/ark_starknet/tests/integration/cancel_order.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
use starknet::{ContractAddress, contract_address_const};

use openzeppelin::token::erc20::interface::{IERC20Dispatcher, IERC20DispatcherTrait};
use openzeppelin::token::erc721::interface::{IERC721Dispatcher, IERC721DispatcherTrait};
use ark_common::protocol::order_v1::OrderV1;
use ark_common::protocol::order_types::{CancelInfo, OrderTrait, RouteType};


use ark_starknet::interfaces::{
IExecutorDispatcher, IExecutorDispatcherTrait, IMaintenanceDispatcher,
IMaintenanceDispatcherTrait
};

use ark_tokens::erc20::{IFreeMintDispatcher, IFreeMintDispatcherTrait};
use ark_tokens::erc721::IFreeMintDispatcher as Erc721Dispatcher;
use ark_tokens::erc721::IFreeMintDispatcherTrait as Erc721DispatcherTrait;

use snforge_std as snf;
use snf::{ContractClass, ContractClassTrait, CheatTarget};

use super::super::common::setup::{setup, setup_order};

fn create_offer_order(
executor_address: ContractAddress,
erc20_address: ContractAddress,
nft_address: ContractAddress,
token_id: u256
) -> (felt252, ContractAddress, u256) {
let offerer = contract_address_const::<'offerer'>();
let start_amount = 10_000_000;

IFreeMintDispatcher { contract_address: erc20_address }.mint(offerer, start_amount);

let mut order = setup_order(erc20_address, nft_address);
order.offerer = offerer;
order.start_amount = start_amount;
order.token_id = Option::Some(token_id);

snf::start_prank(CheatTarget::One(executor_address), offerer);
IExecutorDispatcher { contract_address: executor_address }.create_order(order);
snf::stop_prank(CheatTarget::One(executor_address));

(order.compute_order_hash(), offerer, start_amount)
}

#[test]
fn test_cancel_offer_order() {
let (executor_address, erc20_address, nft_address) = setup();
let token_id = 10;

let (order_hash, offerer, _start_amount) = create_offer_order(
executor_address, erc20_address, nft_address, token_id
);

let cancel_info = CancelInfo {
order_hash,
canceller: offerer,
token_chain_id: 'SN_MAIN',
token_address: nft_address,
token_id: Option::Some(token_id),
};

snf::start_prank(CheatTarget::One(executor_address), offerer);
IExecutorDispatcher { contract_address: executor_address }.cancel_order(cancel_info);
snf::stop_prank(CheatTarget::One(executor_address));
}

#[test]
#[should_panic(expected: ("Caller is not the canceller",))]
fn test_cancel_offer_order_only_offerer() {
let (executor_address, erc20_address, nft_address) = setup();
let token_id = 10;
let other = contract_address_const::<'other'>();

let (order_hash, offerer, _start_amount) = create_offer_order(
executor_address, erc20_address, nft_address, token_id
);

let cancel_info = CancelInfo {
order_hash,
canceller: offerer,
token_chain_id: 'SN_MAIN',
token_address: nft_address,
token_id: Option::Some(token_id),
};

snf::start_prank(CheatTarget::One(executor_address), other);
IExecutorDispatcher { contract_address: executor_address }.cancel_order(cancel_info);
snf::stop_prank(CheatTarget::One(executor_address));
}

#[test]
#[should_panic(expected: ("Canceller is not the offerer",))]
fn test_cancel_offer_order_offerer_is_not_the_canceller() {
let (executor_address, erc20_address, nft_address) = setup();
let token_id = 10;
let other = contract_address_const::<'other'>();

let (order_hash, _offerer, _start_amount) = create_offer_order(
executor_address, erc20_address, nft_address, token_id
);

let cancel_info = CancelInfo {
order_hash,
canceller: other,
token_chain_id: 'SN_MAIN',
token_address: nft_address,
token_id: Option::Some(token_id),
};

snf::start_prank(CheatTarget::One(executor_address), other);
IExecutorDispatcher { contract_address: executor_address }.cancel_order(cancel_info);
snf::stop_prank(CheatTarget::One(executor_address));
}
1 change: 1 addition & 0 deletions contracts/ark_starknet/tests/lib.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod unit {
mod test_fees;
}
mod integration {
mod cancel_order;
mod create_order;
mod execute_order;
mod fees_amount;
Expand Down

0 comments on commit 9fcb990

Please sign in to comment.