Fixing orderbook funds recipient (#247)

CHANGELOG.md

@@ -19,6 +19,10 @@ Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to
 - `Collection` and `Nft` were reverted to use dynamic fields instead of dynamic object fields.
 - Migrated all domains not requiring `key` property after loosening `key` requirement on `Nft` and `Collection` domains.
 
+### Fixed
+
+- Orderbook recipient was wrong when trading in orderbook.
+
 ## [0.22.0] - 2023-02-02
 
 ### Added

sources/trading/orderbook.move

@@ -143,6 +143,7 @@ module nft_protocol::orderbook {
         ///
         /// https://github.com/MystenLabs/sui/issues/2083
         transfer_cap: Option<TransferCap>,
+        seller: address,
         buyer: address,
         buyer_safe: ID,
         paid: Balance<FT>,
@@ -883,6 +884,7 @@ module nft_protocol::orderbook {
             let trade_intermediate = TradeIntermediate<C, FT> {
                 buyer_safe: buyer_safe_id,
                 buyer,
+                seller,
                 commission: ask_commission,
                 id: object::new(ctx),
                 paid: balance::split(coin::balance_mut(wallet), lowest_ask_price),
@@ -1052,6 +1054,7 @@ module nft_protocol::orderbook {
                 id: object::new(ctx),
                 transfer_cap: option::some(transfer_cap),
                 commission: ask_commission,
+                seller,
                 buyer,
                 buyer_safe: buyer_safe_id,
                 paid: bid_offer,
@@ -1148,7 +1151,7 @@ module nft_protocol::orderbook {
 
         let Ask {
             transfer_cap,
-            owner: _,
+            owner: seller,
             price: _,
             commission: maybe_commission,
         } = remove_ask(
@@ -1171,7 +1174,7 @@ module nft_protocol::orderbook {
         let bid_offer = balance::split(coin::balance_mut(wallet), price);
         settle_funds_with_royalties<C, FT>(
             &mut bid_offer,
-            buyer,
+            seller,
             &mut maybe_commission,
             ctx,
         );
@@ -1225,7 +1228,7 @@ module nft_protocol::orderbook {
         let bid_offer = balance::split(coin::balance_mut(wallet), price);
         settle_funds_no_royalties<C, FT>(
             &mut bid_offer,
-            buyer,
+            seller,
             &mut maybe_commission,
             ctx,
         );
@@ -1251,6 +1254,7 @@ module nft_protocol::orderbook {
             id: _,
             transfer_cap,
             paid,
+            seller,
             buyer,
             buyer_safe: expected_buyer_safe_id,
             commission: maybe_commission,
@@ -1267,7 +1271,7 @@ module nft_protocol::orderbook {
 
         settle_funds_with_royalties<C, FT>(
             paid,
-            *buyer,
+            *seller,
             maybe_commission,
             ctx,
         );
@@ -1293,7 +1297,8 @@ module nft_protocol::orderbook {
             id: _,
             transfer_cap,
             paid,
-            buyer,
+            seller,
+            buyer: _,
             buyer_safe: expected_buyer_safe_id,
             commission: maybe_commission,
         } = trade;
@@ -1309,7 +1314,7 @@ module nft_protocol::orderbook {
 
         settle_funds_no_royalties<C, FT>(
             paid,
-            *buyer,
+            *seller,
             maybe_commission,
             ctx,
         );

tests/orderbook/commission.move

@@ -4,14 +4,18 @@ module nft_protocol::test_ob_commission {
     // funds are wrapped in the right way
 
     use nft_protocol::orderbook::{Self as ob, Orderbook};
+    use nft_protocol::royalties;
     use nft_protocol::test_utils as test_ob;
     use sui::sui::SUI;
     use sui::test_scenario;
     use sui::transfer::transfer;
 
+    const BUYER: address = @0xA1C07;
     const SELLER: address = @0xA1C06;
     const CREATOR: address = @0xA1C05;
 
+    const OFFER_SUI: u64 = 100;
+
     #[test]
     #[expected_failure(abort_code = 13370701, location = nft_protocol::orderbook)]
     fun it_cannot_create_ask_with_commission_greater_than_requested_tokens() {
@@ -27,14 +31,58 @@ module nft_protocol::test_ob_commission {
         test_ob::create_ask_with_commission(
             &mut scenario,
             nft_id,
-            100,
+            OFFER_SUI,
             CREATOR,
-            101,
+            OFFER_SUI + 1,
         );
 
         test_scenario::end(scenario);
     }
 
+    #[test]
+    fun it_works() {
+        let scenario = test_scenario::begin(CREATOR);
+
+        test_ob::create_collection_and_allowlist(&mut scenario);
+        let _ob_id = test_ob::create_ob<test_ob::Foo>(&mut scenario);
+        test_scenario::next_tx(&mut scenario, SELLER);
+        test_ob::create_safe(&mut scenario, SELLER);
+        let nft_id = test_ob::create_and_deposit_nft(&mut scenario, SELLER);
+        test_scenario::next_tx(&mut scenario, SELLER);
+
+        test_ob::create_ask_with_commission(
+            &mut scenario,
+            nft_id,
+            OFFER_SUI,
+            CREATOR,
+            10,
+        );
+
+
+        test_ob::create_safe(&mut scenario, BUYER);
+        test_ob::create_bid<test_ob::Foo>(&mut scenario, OFFER_SUI);
+        test_ob::finish_trade(
+            &mut scenario,
+            nft_id,
+            BUYER,
+            SELLER,
+        );
+
+        test_scenario::next_tx(&mut scenario, SELLER);
+        let payment_for_commission: royalties::TradePayment<test_ob::Foo, SUI> =
+            test_scenario::take_shared(&mut scenario);
+        assert!(royalties::beneficiary(&payment_for_commission) == CREATOR, 0);
+
+        let payment_for_seller: royalties::TradePayment<test_ob::Foo, SUI> =
+            test_scenario::take_shared(&mut scenario);
+        assert!(royalties::beneficiary(&payment_for_seller) == SELLER, 0);
+
+        test_scenario::return_shared(payment_for_commission);
+        test_scenario::return_shared(payment_for_seller);
+
+        test_scenario::end(scenario);
+    }
+
     #[test]
     fun it_lists_nft() {
         let scenario = test_scenario::begin(CREATOR);

tests/orderbook/safe_to_safe_trade.move

@@ -9,6 +9,7 @@ module nft_protocol::test_ob_safe_to_safe_trade {
     use originmate::box::Box;
     use sui::coin::{Self, Coin};
     use sui::sui::SUI;
+    use nft_protocol::royalties;
     use sui::test_scenario;
 
     const BUYER: address = @0xA1C06;
@@ -45,6 +46,12 @@ module nft_protocol::test_ob_safe_to_safe_trade {
             SELLER,
         );
 
+        test_scenario::next_tx(&mut scenario, SELLER);
+        let payment_for_seller: royalties::TradePayment<test_ob::Foo, SUI> =
+            test_scenario::take_shared(&mut scenario);
+        assert!(royalties::beneficiary(&payment_for_seller) == SELLER, 0);
+        test_scenario::return_shared(payment_for_seller);
+
         test_scenario::end(scenario);
     }
 
@@ -73,7 +80,7 @@ module nft_protocol::test_ob_safe_to_safe_trade {
             SELLER,
         );
 
-        test_scenario::next_tx(&mut scenario, BUYER);
+        test_scenario::next_tx(&mut scenario, SELLER);
         let offer: Coin<SUI> = test_scenario::take_from_sender(&mut scenario);
         assert!(coin::value(&offer) == OFFER_SUI, 0);
         test_scenario::return_to_sender(&mut scenario, offer);