From d81ed8d15c6e69b447d421f727963e48c44a9d62 Mon Sep 17 00:00:00 2001 From: ptisserand Date: Thu, 24 Oct 2024 00:59:02 +0200 Subject: [PATCH] fix: emit OrderCancelled event when an order is cancelled by a new one (#486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description - Add missing emit of `OrderCancelled` event when an order is cancelled by a new one. - Fix local deployment - SDK: add missing `quantity` field in `FulfillInfo` ## 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 ## Added tests? - [x] 👍 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? ## [optional] What gif best describes this PR or how it makes you feel? ### 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 --- .../src/orderbook/orderbook.cairo | 24 +++ .../tests/integration/create_order.cairo | 194 +++++++++++++++++- .../core/src/actions/order/fulfillAuction.ts | 3 + .../core/src/actions/order/fulfillListing.ts | 3 + .../core/src/actions/order/fulfillOffer.ts | 3 + packages/core/src/types/index.ts | 1 + packages/core/tests/fulfillAuction.test.ts | 3 +- packages/core/tests/fulfillListing.test.ts | 1 + packages/core/tests/fulfillOffer.test.ts | 4 +- .../deployer/src/contracts/erc721royalties.ts | 2 +- 10 files changed, 234 insertions(+), 4 deletions(-) diff --git a/contracts/ark_component/src/orderbook/orderbook.cairo b/contracts/ark_component/src/orderbook/orderbook.cairo index 0ed799812..295041339 100644 --- a/contracts/ark_component/src/orderbook/orderbook.cairo +++ b/contracts/ark_component/src/orderbook/orderbook.cairo @@ -666,6 +666,18 @@ pub mod OrderbookComponent { let cancelled_order_hash = self._process_previous_order(token_hash, order.offerer); order_write(order_hash, order_type, order); self.token_listings.write(token_hash, order_hash); + if (cancelled_order_hash.is_some()) { + let cancelled_order_hash = cancelled_order_hash.unwrap(); + self + .emit( + OrderCancelled { + order_hash: cancelled_order_hash, + reason: OrderStatus::CancelledByNewOrder.into(), + order_type, + version: ORDER_CANCELLED_EVENT_VERSION, + } + ) + } self .emit( OrderPlaced { @@ -710,6 +722,18 @@ pub mod OrderbookComponent { let cancelled_order_hash = self._process_previous_order(token_hash, order.offerer); order_write(order_hash, order_type, order); self.auctions.write(token_hash, (order_hash, order.end_date, 0)); + if (cancelled_order_hash.is_some()) { + let cancelled_order_hash = cancelled_order_hash.unwrap(); + self + .emit( + OrderCancelled { + order_hash: cancelled_order_hash, + reason: OrderStatus::CancelledByNewOrder.into(), + order_type, + version: ORDER_CANCELLED_EVENT_VERSION, + } + ) + } self .emit( OrderPlaced { diff --git a/contracts/ark_starknet/tests/integration/create_order.cairo b/contracts/ark_starknet/tests/integration/create_order.cairo index 1ae604818..1c8a470f0 100644 --- a/contracts/ark_starknet/tests/integration/create_order.cairo +++ b/contracts/ark_starknet/tests/integration/create_order.cairo @@ -18,6 +18,8 @@ use ark_tokens::erc20::IFreeMintDispatcherTrait as Erc20DispatcherTrait; use ark_tokens::erc721::IFreeMintDispatcher as Erc721Dispatcher; use ark_tokens::erc721::IFreeMintDispatcherTrait as Erc721DispatcherTrait; +use openzeppelin::token::erc721::interface::{IERC721Dispatcher, IERC721DispatcherTrait}; + use snforge_std::{ cheat_caller_address, CheatSpan, spy_events, EventSpyAssertionsTrait, start_cheat_block_timestamp_global, stop_cheat_block_timestamp_global @@ -523,7 +525,6 @@ fn test_create_order_erc1155_to_erc20_ok() { fn test_create_order_offerer_not_own_enough_erc1155_token() { let (executor_address, erc20_address, erc1155_address) = setup_erc1155(); let offerer = erc1155_address; - let other = contract_address_const::<'other'>(); let quantity = 50_u256; let token_id = Erc1155Dispatcher { contract_address: erc1155_address }.mint(offerer, 2_u256); @@ -582,3 +583,194 @@ fn test_create_order_erc1155_to_erc20_disabled() { IExecutorDispatcher { contract_address: executor_address }.create_order(order); } + +#[test] +fn test_create_listing_order_transfer_token_to_other_user() { + let (executor_address, erc20_address, nft_address) = setup(); + let offerer = contract_address_const::<'offerer'>(); + let other_offerer = contract_address_const::<'other_offerer'>(); + + let start_amount = 10_000_000; + let other_start_amount = 20_000_000; + + let token_id: u256 = Erc721Dispatcher { contract_address: nft_address } + .get_current_token_id() + .into(); + Erc721Dispatcher { contract_address: nft_address }.mint(offerer, 'base_uri'); + + let order = setup_listing_order(erc20_address, nft_address, offerer, token_id, start_amount); + let order_hash = order.compute_order_hash(); + + cheat_caller_address(executor_address, offerer, CheatSpan::TargetCalls(1)); + IExecutorDispatcher { contract_address: executor_address }.create_order(order); + + cheat_caller_address(nft_address, offerer, CheatSpan::TargetCalls(1)); + IERC721Dispatcher { contract_address: nft_address } + .transfer_from(offerer, other_offerer, token_id); + + let other_order = setup_listing_order( + erc20_address, nft_address, other_offerer, token_id, other_start_amount + ); + let other_order_hash = other_order.compute_order_hash(); + let order_version = other_order.get_version(); + + let mut spy = spy_events(); + cheat_caller_address(executor_address, other_offerer, CheatSpan::TargetCalls(1)); + IExecutorDispatcher { contract_address: executor_address }.create_order(other_order); + + spy + .assert_emitted( + @array![ + ( + executor_address, + executor::Event::OrderbookEvent( + OrderbookComponent::Event::OrderCancelled( + OrderbookComponent::OrderCancelled { + order_hash, + reason: OrderStatus::CancelledByNewOrder.into(), + order_type: OrderType::Listing, + version: OrderbookComponent::ORDER_CANCELLED_EVENT_VERSION, + } + ) + ) + ), + ( + executor_address, + executor::Event::OrderbookEvent( + OrderbookComponent::Event::OrderPlaced( + OrderbookComponent::OrderPlaced { + order_hash: other_order_hash, + order_version, + order_type: OrderType::Listing, + version: OrderbookComponent::ORDER_PLACED_EVENT_VERSION, + cancelled_order_hash: Option::Some(order_hash), + order: other_order, + } + ) + ) + ) + ] + ); + + assert_eq!( + IOrderbookDispatcher { contract_address: executor_address }.get_order_type(order_hash), + OrderType::Listing, + "Wrong order type" + ); + assert_eq!( + IOrderbookDispatcher { contract_address: executor_address } + .get_order_type(other_order_hash), + OrderType::Listing, + "Wrong other order type" + ); + + assert_eq!( + IOrderbookDispatcher { contract_address: executor_address }.get_order_status(order_hash), + OrderStatus::CancelledByNewOrder, + "Wrong order status" + ); + + assert_eq!( + IOrderbookDispatcher { contract_address: executor_address } + .get_order_status(other_order_hash), + OrderStatus::Open, + "Wrong other order status" + ); +} + +#[test] +fn test_create_auction_order_transfer_token_to_other_user() { + let (executor_address, erc20_address, nft_address) = setup(); + let offerer = contract_address_const::<'offerer'>(); + let other_offerer = contract_address_const::<'other_offerer'>(); + + let start_amount = 10_000_000; + let other_start_amount = 15_000_000; + let end_amount = start_amount * 2; + + let token_id: u256 = Erc721Dispatcher { contract_address: nft_address } + .get_current_token_id() + .into(); + Erc721Dispatcher { contract_address: nft_address }.mint(offerer, 'base_uri'); + + let order = setup_auction_order( + erc20_address, nft_address, offerer, token_id, start_amount, end_amount + ); + let order_hash = order.compute_order_hash(); + + cheat_caller_address(executor_address, offerer, CheatSpan::TargetCalls(1)); + IExecutorDispatcher { contract_address: executor_address }.create_order(order); + + cheat_caller_address(nft_address, offerer, CheatSpan::TargetCalls(1)); + IERC721Dispatcher { contract_address: nft_address } + .transfer_from(offerer, other_offerer, token_id); + + let other_order = setup_auction_order( + erc20_address, nft_address, other_offerer, token_id, other_start_amount, end_amount + ); + let other_order_hash = other_order.compute_order_hash(); + let order_version = other_order.get_version(); + + let mut spy = spy_events(); + cheat_caller_address(executor_address, other_offerer, CheatSpan::TargetCalls(1)); + IExecutorDispatcher { contract_address: executor_address }.create_order(other_order); + + spy + .assert_emitted( + @array![ + ( + executor_address, + executor::Event::OrderbookEvent( + OrderbookComponent::Event::OrderCancelled( + OrderbookComponent::OrderCancelled { + order_hash, + reason: OrderStatus::CancelledByNewOrder.into(), + order_type: OrderType::Auction, + version: OrderbookComponent::ORDER_CANCELLED_EVENT_VERSION, + } + ) + ) + ), + ( + executor_address, + executor::Event::OrderbookEvent( + OrderbookComponent::Event::OrderPlaced( + OrderbookComponent::OrderPlaced { + order_hash: other_order_hash, + order_version, + order_type: OrderType::Auction, + version: OrderbookComponent::ORDER_PLACED_EVENT_VERSION, + cancelled_order_hash: Option::Some(order_hash), + order: other_order, + } + ) + ) + ) + ] + ); + + assert_eq!( + IOrderbookDispatcher { contract_address: executor_address }.get_order_type(order_hash), + OrderType::Auction, + "Wrong order type" + ); + assert_eq!( + IOrderbookDispatcher { contract_address: executor_address } + .get_order_type(other_order_hash), + OrderType::Auction, + "Wrong other order type" + ); + + assert_eq!( + IOrderbookDispatcher { contract_address: executor_address }.get_order_status(order_hash), + OrderStatus::CancelledByNewOrder, + "Wrong order status" + ); + + assert_eq!( + IOrderbookDispatcher { contract_address: executor_address } + .get_order_status(other_order_hash), + OrderStatus::Open, + "Wrong other order status" + ); +} diff --git a/packages/core/src/actions/order/fulfillAuction.ts b/packages/core/src/actions/order/fulfillAuction.ts index 3ea4f2a08..3c5abb294 100644 --- a/packages/core/src/actions/order/fulfillAuction.ts +++ b/packages/core/src/actions/order/fulfillAuction.ts @@ -17,6 +17,7 @@ export interface FulfillAuctionParameters { relatedOrderHash: bigint; tokenAddress: string; tokenId: bigint; + quantity: bigint; currencyAddress?: string; waitForTransaction?: boolean; } @@ -44,6 +45,7 @@ export async function fulfillAuction( relatedOrderHash, tokenAddress, tokenId, + quantity, waitForTransaction = true } = parameters; const chainId = await config.starknetProvider.getChainId(); @@ -61,6 +63,7 @@ export async function fulfillAuction( CairoOptionVariant.Some, cairo.uint256(tokenId) ), + quantity: cairo.uint256(quantity), fulfillBrokerAddress: brokerAddress }; diff --git a/packages/core/src/actions/order/fulfillListing.ts b/packages/core/src/actions/order/fulfillListing.ts index cb270f309..f3a7b2219 100644 --- a/packages/core/src/actions/order/fulfillListing.ts +++ b/packages/core/src/actions/order/fulfillListing.ts @@ -18,6 +18,7 @@ export interface FulfillListingParameters { orderHash: bigint; tokenAddress: string; tokenId: bigint; + quantity: bigint; amount: bigint; waitForTransaction?: boolean; } @@ -45,6 +46,7 @@ export async function fulfillListing( orderHash, tokenAddress, tokenId, + quantity, amount, waitForTransaction = true } = parameters; @@ -66,6 +68,7 @@ export async function fulfillListing( CairoOptionVariant.Some, cairo.uint256(tokenId) ), + quantity: cairo.uint256(quantity), fulfillBrokerAddress: brokerAddress }; diff --git a/packages/core/src/actions/order/fulfillOffer.ts b/packages/core/src/actions/order/fulfillOffer.ts index 157cb2cd5..96937c685 100644 --- a/packages/core/src/actions/order/fulfillOffer.ts +++ b/packages/core/src/actions/order/fulfillOffer.ts @@ -16,6 +16,7 @@ export interface FulfillOfferParameters { orderHash: bigint; tokenAddress: string; tokenId: bigint; + quantity: bigint; waitForTransaction?: boolean; } @@ -41,6 +42,7 @@ export async function fulfillOffer( orderHash, tokenAddress, tokenId, + quantity, waitForTransaction = true } = parameters; const chainId = await config.starknetProvider.getChainId(); @@ -55,6 +57,7 @@ export async function fulfillOffer( CairoOptionVariant.Some, cairo.uint256(tokenId) ), + quantity: cairo.uint256(quantity), fulfillBrokerAddress: brokerAddress }; diff --git a/packages/core/src/types/index.ts b/packages/core/src/types/index.ts index 3d73c91c1..0ca2c4c79 100644 --- a/packages/core/src/types/index.ts +++ b/packages/core/src/types/index.ts @@ -91,6 +91,7 @@ export type FulfillInfo = { tokenChainId: constants.StarknetChainId; tokenAddress: string; tokenId: CairoOption; + quantity: Uint256; fulfillBrokerAddress: string; }; diff --git a/packages/core/tests/fulfillAuction.test.ts b/packages/core/tests/fulfillAuction.test.ts index 9b5e00784..d62643e56 100644 --- a/packages/core/tests/fulfillAuction.test.ts +++ b/packages/core/tests/fulfillAuction.test.ts @@ -42,7 +42,8 @@ describe("fulfillAuction", () => { orderHash, relatedOrderHash: offerOrderHash, tokenAddress, - tokenId + tokenId, + quantity: BigInt(1), }); const { orderStatus } = await getOrderStatus(config, { diff --git a/packages/core/tests/fulfillListing.test.ts b/packages/core/tests/fulfillListing.test.ts index ffad87486..d142e3384 100644 --- a/packages/core/tests/fulfillListing.test.ts +++ b/packages/core/tests/fulfillListing.test.ts @@ -25,6 +25,7 @@ describe("fulfillOffer", () => { orderHash, tokenAddress, tokenId, + quantity: BigInt(1), amount: startAmount }); diff --git a/packages/core/tests/fulfillOffer.test.ts b/packages/core/tests/fulfillOffer.test.ts index c07ad7d07..570b779bc 100644 --- a/packages/core/tests/fulfillOffer.test.ts +++ b/packages/core/tests/fulfillOffer.test.ts @@ -31,7 +31,8 @@ describe("fulfillOffer", () => { brokerAddress: saleBroker.address, orderHash, tokenAddress, - tokenId + tokenId, + quantity: BigInt(1), }); const { orderStatus: orderStatusFulfilled } = await getOrderStatus(config, { @@ -66,6 +67,7 @@ describe("fulfillOffer", () => { orderHash, tokenAddress, tokenId, + quantity: BigInt(1), brokerAddress: saleBroker.address }); diff --git a/packages/deployer/src/contracts/erc721royalties.ts b/packages/deployer/src/contracts/erc721royalties.ts index 00a48c5ab..a124e51ef 100644 --- a/packages/deployer/src/contracts/erc721royalties.ts +++ b/packages/deployer/src/contracts/erc721royalties.ts @@ -48,7 +48,7 @@ export async function deployERC721Royalties( constructorCalldata: contractConstructor }); - setDefaultFees( + await setDefaultFees( provider, deployerAccount, deployR.deploy.contract_address,