From 4253f43fb1a428e0f4b639a94ea995c9e71809a5 Mon Sep 17 00:00:00 2001 From: ignazio Date: Wed, 18 May 2022 15:58:11 +0200 Subject: [PATCH 01/17] add failing test on auction with member owned channel with no reward --- .../src/tests/nft/pick_open_auction_winner.rs | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/runtime-modules/content/src/tests/nft/pick_open_auction_winner.rs b/runtime-modules/content/src/tests/nft/pick_open_auction_winner.rs index e1bff9974d..011409b036 100644 --- a/runtime-modules/content/src/tests/nft/pick_open_auction_winner.rs +++ b/runtime-modules/content/src/tests/nft/pick_open_auction_winner.rs @@ -605,3 +605,71 @@ fn auction_proceeds_are_burned_in_case_of_curator_owned_channel_is_auctioneer() assert_eq!(Balances::::total_issuance(), issuance_pre - bid); }) } + +#[test] +fn auction_proceeds_ok_in_case_of_member_owned_channel_with_no_reward_account_is_auctioneer() { + with_default_mock_builder(|| { + let video_id = Content::next_video_id(); + CreateChannelFixture::default() + .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) + .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) + .call(); + CreateVideoFixture::default() + .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) + .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) + .call(); + + assert_ok!(Content::issue_nft( + Origin::signed(DEFAULT_MEMBER_ACCOUNT_ID), + ContentActor::Member(DEFAULT_MEMBER_ID), + video_id, + NftIssuanceParameters::::default(), + )); + + let bid_lock_duration = Content::min_bid_lock_duration(); + + let auction_params = OpenAuctionParams:: { + starting_price: Content::min_starting_price(), + buy_now_price: None, + bid_lock_duration, + starts_at: None, + whitelist: BTreeSet::new(), + }; + + // Start nft auction + assert_ok!(Content::start_open_auction( + Origin::signed(DEFAULT_MEMBER_ACCOUNT_ID), + ContentActor::Member(DEFAULT_MEMBER_ID), + video_id, + auction_params.clone(), + )); + + // deposit initial balance + let bid = Content::min_starting_price(); + let platform_fee = Content::platform_fee_percentage().mul_floor(bid); + + let _ = balances::Module::::deposit_creating(&SECOND_MEMBER_ACCOUNT_ID, bid); + + // Make nft auction bid + assert_ok!(Content::make_open_auction_bid( + Origin::signed(SECOND_MEMBER_ACCOUNT_ID), + SECOND_MEMBER_ID, + video_id, + bid, + )); + + // Pick open auction winner + assert_ok!(Content::pick_open_auction_winner( + Origin::signed(DEFAULT_MEMBER_ACCOUNT_ID), + ContentActor::Member(DEFAULT_MEMBER_ID), + video_id, + SECOND_MEMBER_ID, + bid, + )); + + assert_eq!( + Balances::::usable_balance(DEFAULT_MEMBER_ACCOUNT_ID), + bid - platform_fee + ); + }) +} From 5fa6c3764f73bb664e7e00ca6f0d2fb6605a6b7a Mon Sep 17 00:00:00 2001 From: ignazio Date: Wed, 18 May 2022 16:27:53 +0200 Subject: [PATCH 02/17] all tests passing --- runtime-modules/content/src/nft/mod.rs | 20 +++++++++-- .../src/tests/nft/accept_incoming_offer.rs | 36 ++++++++++--------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/runtime-modules/content/src/nft/mod.rs b/runtime-modules/content/src/nft/mod.rs index 1540dd1573..5ccf21da01 100644 --- a/runtime-modules/content/src/nft/mod.rs +++ b/runtime-modules/content/src/nft/mod.rs @@ -382,15 +382,29 @@ impl Module { .with_member_owner(winner_id) } + /// Channel reward account is + /// - Some -> use that as reward account + /// - None -> then if channel owner is: + /// - Member -> use member controller account + /// - CuratorGroup -> burn pub(crate) fn ensure_owner_account_id( channel_id: T::ChannelId, nft: &Nft, ) -> Result { match nft.owner { NftOwner::Member(member_id) => T::MemberAuthenticator::controller_account_id(member_id), - NftOwner::ChannelOwner => Self::channel_by_id(channel_id) - .reward_account - .ok_or_else(|| Error::::RewardAccountIsNotSet.into()), + NftOwner::ChannelOwner => { + let channel = Self::ensure_channel_exists(&channel_id)?; + channel.reward_account.as_ref().map_or_else( + || match channel.owner { + ChannelOwner::Member(member_id) => { + T::MemberAuthenticator::controller_account_id(member_id) + } + _ => Err(Error::::RewardAccountIsNotSet.into()), + }, + |ra| Ok(ra.to_owned()), + ) + } } } diff --git a/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs b/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs index f2d3691731..8753fe4fb8 100644 --- a/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs +++ b/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs @@ -1,7 +1,9 @@ #![cfg(test)] +use crate::tests::curators; use crate::tests::fixtures::{ create_default_member_owned_channel_with_video, create_initial_storage_buckets_helper, - increase_account_balance_helper, UpdateChannelFixture, + increase_account_balance_helper, CreateChannelFixture, CreateVideoFixture, + UpdateChannelFixture, }; use crate::tests::mock::*; use crate::*; @@ -228,31 +230,31 @@ fn accept_incoming_offer_reward_account_burns_token_with_curator_owner_channel() // Run to block one to see emitted events run_to_block(1); - let video_id = NextVideoId::::get(); - create_initial_storage_buckets_helper(); - increase_account_balance_helper(DEFAULT_MEMBER_ACCOUNT_ID, INITIAL_BALANCE); - increase_account_balance_helper(SECOND_MEMBER_ACCOUNT_ID, DEFAULT_NFT_PRICE); - create_default_member_owned_channel_with_video(); + let curator_group_id = curators::add_curator_to_new_group(DEFAULT_CURATOR_ID); + let video_id = Content::next_video_id(); + let curator_actor = ContentActor::Curator(curator_group_id, DEFAULT_CURATOR_ID); + CreateChannelFixture::default() + .with_sender(DEFAULT_CURATOR_ACCOUNT_ID) + .with_actor(curator_actor) + .call(); + CreateVideoFixture::default() + .with_sender(DEFAULT_CURATOR_ACCOUNT_ID) + .with_actor(ContentActor::Curator(curator_group_id, DEFAULT_CURATOR_ID)) + .call(); - UpdateChannelFixture::default() - .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) - .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) - .with_reward_account(Some(None)) - .call_and_assert(Ok(())); - - // Issue nft assert_ok!(Content::issue_nft( - Origin::signed(DEFAULT_MEMBER_ACCOUNT_ID), - ContentActor::Member(DEFAULT_MEMBER_ID), + Origin::signed(DEFAULT_CURATOR_ACCOUNT_ID), + curator_actor, video_id, NftIssuanceParameters::::default(), )); + increase_account_balance_helper(SECOND_MEMBER_ACCOUNT_ID, DEFAULT_NFT_PRICE); // Offer nft assert_ok!(Content::offer_nft( - Origin::signed(DEFAULT_MEMBER_ACCOUNT_ID), + Origin::signed(DEFAULT_CURATOR_ACCOUNT_ID), video_id, - ContentActor::Member(DEFAULT_MEMBER_ID), + curator_actor, SECOND_MEMBER_ID, Some(DEFAULT_NFT_PRICE), )); From 3625ce24e4f7478fb9818c9f0b6aea9887c8edf9 Mon Sep 17 00:00:00 2001 From: ignazio Date: Wed, 18 May 2022 16:41:49 +0200 Subject: [PATCH 03/17] add test for buy_now and accept_incoming offer case : NFT is member-channel owned and reward account is not set --- .../src/tests/nft/accept_incoming_offer.rs | 38 ++++++++++--------- .../content/src/tests/nft/buy_nft.rs | 31 +++++++++++++++ 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs b/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs index 8753fe4fb8..df41032ca0 100644 --- a/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs +++ b/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs @@ -3,7 +3,6 @@ use crate::tests::curators; use crate::tests::fixtures::{ create_default_member_owned_channel_with_video, create_initial_storage_buckets_helper, increase_account_balance_helper, CreateChannelFixture, CreateVideoFixture, - UpdateChannelFixture, }; use crate::tests::mock::*; use crate::*; @@ -182,23 +181,21 @@ fn accept_incoming_offer_no_incoming_offers() { } #[test] -fn accept_incoming_offer_reward_account_is_not_set_succeeds_with_member_owner_channel() { +fn accept_incoming_offer_ok_with_reward_account_is_not_set_succeeds_with_member_owner_channel() { + let video_id = 1u64; with_default_mock_builder(|| { // Run to block one to see emitted events run_to_block(1); - - let video_id = NextVideoId::::get(); - create_initial_storage_buckets_helper(); - increase_account_balance_helper(DEFAULT_MEMBER_ACCOUNT_ID, INITIAL_BALANCE); - create_default_member_owned_channel_with_video(); - - UpdateChannelFixture::default() + // channel with no reward account + CreateChannelFixture::default() + .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) + .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) + .call(); + CreateVideoFixture::default() .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) - .with_reward_account(Some(None)) - .call_and_assert(Ok(())); + .call(); - // Issue nft assert_ok!(Content::issue_nft( Origin::signed(DEFAULT_MEMBER_ACCOUNT_ID), ContentActor::Member(DEFAULT_MEMBER_ID), @@ -206,21 +203,28 @@ fn accept_incoming_offer_reward_account_is_not_set_succeeds_with_member_owner_ch NftIssuanceParameters::::default(), )); + increase_account_balance_helper(SECOND_MEMBER_ACCOUNT_ID, DEFAULT_NFT_PRICE); + // Offer nft assert_ok!(Content::offer_nft( Origin::signed(DEFAULT_MEMBER_ACCOUNT_ID), video_id, ContentActor::Member(DEFAULT_MEMBER_ID), SECOND_MEMBER_ID, - None, + Some(100u64), // price )); // Make an attempt to accept incoming nft offer if sender is owner and reward account is not set - let accept_incoming_offer_result = - Content::accept_incoming_offer(Origin::signed(SECOND_MEMBER_ACCOUNT_ID), video_id); + assert_ok!(Content::accept_incoming_offer( + Origin::signed(SECOND_MEMBER_ACCOUNT_ID), + video_id + )); - // Failure checked - assert_ok!(accept_incoming_offer_result,); + // check owner balance increased by net profit + assert_eq!( + Balances::::usable_balance(DEFAULT_MEMBER_ACCOUNT_ID), + 100u64 - (Content::platform_fee_percentage() * 100u64) + ); }) } diff --git a/runtime-modules/content/src/tests/nft/buy_nft.rs b/runtime-modules/content/src/tests/nft/buy_nft.rs index f2ee37d029..33c5134147 100644 --- a/runtime-modules/content/src/tests/nft/buy_nft.rs +++ b/runtime-modules/content/src/tests/nft/buy_nft.rs @@ -577,6 +577,37 @@ pub fn proceeds_are_burned_if_nft_owned_by_curator_channel_with_no_reward_accoun }) } +#[test] +pub fn proceeds_are_correctly_credited_if_nft_owned_by_member_channel_with_no_reward_account_and_royalty_specified( +) { + with_default_mock_builder(|| { + increase_account_balance_helper(SECOND_MEMBER_ACCOUNT_ID, 100u64); + // channel with no reward account + CreateChannelFixture::default() + .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) + .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) + .call(); + CreateVideoFixture::default() + .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) + .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) + .with_nft_in_sale(100u64) + .with_nft_royalty(1) + .call(); + + assert_ok!(Content::buy_nft( + Origin::signed(SECOND_MEMBER_ACCOUNT_ID), + 1u64, + SECOND_MEMBER_ID, + 100u64, + )); + + assert_eq!( + Balances::::usable_balance(DEFAULT_MEMBER_ACCOUNT_ID), + 100u64 - (Content::platform_fee_percentage() * 100u64) + ); + }) +} + #[test] pub fn nft_channel_member_owner_is_correctly_credited_after_sale() { with_default_mock_builder(|| { From 5517579ba94c08d284cd714f6331a4416331d009 Mon Sep 17 00:00:00 2001 From: ignazio Date: Wed, 18 May 2022 16:48:52 +0200 Subject: [PATCH 04/17] fix doc line --- runtime-modules/content/src/nft/mod.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/runtime-modules/content/src/nft/mod.rs b/runtime-modules/content/src/nft/mod.rs index 5ccf21da01..d9d6fc2bc9 100644 --- a/runtime-modules/content/src/nft/mod.rs +++ b/runtime-modules/content/src/nft/mod.rs @@ -382,11 +382,13 @@ impl Module { .with_member_owner(winner_id) } - /// Channel reward account is - /// - Some -> use that as reward account - /// - None -> then if channel owner is: - /// - Member -> use member controller account - /// - CuratorGroup -> burn + /// NFT owned by: + /// - Member: member controller account is used + /// - Channel: then if reward account is: + /// - `Some(acc)` -> use `acc` as reward account + /// - `None` -> then if channel owner is: + /// - `Member` -> use member controller account + /// - `CuratorGroup` -> Error pub(crate) fn ensure_owner_account_id( channel_id: T::ChannelId, nft: &Nft, From 5864c8cc3706640c5d414e9854f4df74533ac364 Mon Sep 17 00:00:00 2001 From: ignazio Date: Wed, 18 May 2022 17:27:21 +0200 Subject: [PATCH 05/17] use ensure_reward_account instead --- runtime-modules/content/src/lib.rs | 4 ++-- runtime-modules/content/src/nft/mod.rs | 14 +++----------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/runtime-modules/content/src/lib.rs b/runtime-modules/content/src/lib.rs index 4bfff099b7..3bf018f0e2 100644 --- a/runtime-modules/content/src/lib.rs +++ b/runtime-modules/content/src/lib.rs @@ -2046,7 +2046,7 @@ decl_module! { Self::ensure_new_pending_offer_available_to_proceed(&nft, &receiver_account_id)?; // account_id where the nft offer price is deposited - let nft_owner_account = Self::ensure_owner_account_id(video.in_channel, &nft).ok(); + let nft_owner_account = Self::ensure_nft_owner_account_id(video.in_channel, &nft).ok(); // // == MUTATION SAFE == // @@ -2122,7 +2122,7 @@ decl_module! { Self::ensure_can_buy_now(&nft, &participant_account_id, price_commit)?; // seller account - let old_nft_owner_account_id = Self::ensure_owner_account_id(video.in_channel, &nft).ok(); + let old_nft_owner_account_id = Self::ensure_nft_owner_account_id(video.in_channel, &nft).ok(); // // == MUTATION SAFE == diff --git a/runtime-modules/content/src/nft/mod.rs b/runtime-modules/content/src/nft/mod.rs index d9d6fc2bc9..a912e5956f 100644 --- a/runtime-modules/content/src/nft/mod.rs +++ b/runtime-modules/content/src/nft/mod.rs @@ -368,7 +368,7 @@ impl Module { winner_id: T::MemberId, amount: BalanceOf, ) -> Nft { - let account_deposit_into = Self::ensure_owner_account_id(in_channel, &nft).ok(); + let account_deposit_into = Self::ensure_nft_owner_account_id(in_channel, &nft).ok(); let account_withdraw_from = ContentTreasury::::module_account_id(); Self::complete_payment( @@ -389,7 +389,7 @@ impl Module { /// - `None` -> then if channel owner is: /// - `Member` -> use member controller account /// - `CuratorGroup` -> Error - pub(crate) fn ensure_owner_account_id( + pub(crate) fn ensure_nft_owner_account_id( channel_id: T::ChannelId, nft: &Nft, ) -> Result { @@ -397,15 +397,7 @@ impl Module { NftOwner::Member(member_id) => T::MemberAuthenticator::controller_account_id(member_id), NftOwner::ChannelOwner => { let channel = Self::ensure_channel_exists(&channel_id)?; - channel.reward_account.as_ref().map_or_else( - || match channel.owner { - ChannelOwner::Member(member_id) => { - T::MemberAuthenticator::controller_account_id(member_id) - } - _ => Err(Error::::RewardAccountIsNotSet.into()), - }, - |ra| Ok(ra.to_owned()), - ) + Self::ensure_reward_account(&channel) } } } From 2ef022e8b9767aeb03f11ef279c8b7b2150420fe Mon Sep 17 00:00:00 2001 From: ignazio Date: Wed, 18 May 2022 18:36:32 +0200 Subject: [PATCH 06/17] refactor reward payment methods & review tests --- runtime-modules/content/src/nft/mod.rs | 17 ++-------- .../src/tests/nft/accept_incoming_offer.rs | 2 +- .../content/src/tests/nft/buy_nft.rs | 32 +++++++++++-------- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/runtime-modules/content/src/nft/mod.rs b/runtime-modules/content/src/nft/mod.rs index a912e5956f..8d8f0a5e52 100644 --- a/runtime-modules/content/src/nft/mod.rs +++ b/runtime-modules/content/src/nft/mod.rs @@ -470,20 +470,9 @@ impl Module { // payment is none if there is no royalty if let Some(royalty) = creator_royalty { let channel = Self::channel_by_id(&video.in_channel); - // use reward account if specified - if let Some(creator_reward_account) = channel.reward_account { - Some((royalty, creator_reward_account)) - } else { - // otherwise resort to controller account for member owned channels - if let ChannelOwner::Member(member_id) = channel.owner { - T::MemberAuthenticator::controller_account_id(member_id) - .ok() - .map(|reward_account| (royalty, reward_account)) - } else { - // no royalty paid for curator owned channel with unspecified reward account - None - } - } + Self::ensure_reward_account(&channel) + .ok() + .map(|reward_acc| (royalty, reward_acc)) } else { None } diff --git a/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs b/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs index df41032ca0..b670b09021 100644 --- a/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs +++ b/runtime-modules/content/src/tests/nft/accept_incoming_offer.rs @@ -181,7 +181,7 @@ fn accept_incoming_offer_no_incoming_offers() { } #[test] -fn accept_incoming_offer_ok_with_reward_account_is_not_set_succeeds_with_member_owner_channel() { +fn accept_incoming_offer_ok_with_reward_account_not_set_succeeds_with_member_owner_channel() { let video_id = 1u64; with_default_mock_builder(|| { // Run to block one to see emitted events diff --git a/runtime-modules/content/src/tests/nft/buy_nft.rs b/runtime-modules/content/src/tests/nft/buy_nft.rs index 33c5134147..ffa31e4e52 100644 --- a/runtime-modules/content/src/tests/nft/buy_nft.rs +++ b/runtime-modules/content/src/tests/nft/buy_nft.rs @@ -463,28 +463,35 @@ fn buy_now_ok_with_nft_owner_member_credited_with_payment() { } #[test] -fn buy_now_ok_with_nft_owner_member_owned_channel_credited_with_payment() { +fn buy_now_ok_with_nft_owner_member_owned_channel_and_no_reward_account_credited_with_payment() { with_default_mock_builder(|| { // Run to block one to see emitted events - let starting_block = 1; - let video_id = Content::next_video_id(); - run_to_block(starting_block); - setup_nft_on_sale_scenario(); - increase_account_balance_helper(SECOND_MEMBER_ACCOUNT_ID, DEFAULT_NFT_PRICE); - let platform_fee = Content::platform_fee_percentage().mul_floor(DEFAULT_NFT_PRICE); - let balance_pre = Balances::::usable_balance(DEFAULT_MEMBER_ACCOUNT_ID); + run_to_block(1u64); + + let video_id = 1u64; + CreateChannelFixture::default() + .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) + .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) + .call_and_assert(Ok(())); + CreateVideoFixture::default() + .with_sender(DEFAULT_MEMBER_ACCOUNT_ID) + .with_actor(ContentActor::Member(DEFAULT_MEMBER_ID)) + .with_channel_id(1u64) + .with_nft_in_sale(100u64) + .call_and_assert(Ok(())); + + increase_account_balance_helper(SECOND_MEMBER_ACCOUNT_ID, 100u64); assert_ok!(Content::buy_nft( Origin::signed(SECOND_MEMBER_ACCOUNT_ID), video_id, SECOND_MEMBER_ID, - DEFAULT_NFT_PRICE, + 100u64, )); assert_eq!( Balances::::usable_balance(DEFAULT_MEMBER_ACCOUNT_ID), - // balance_pre - platform fee (since channel owner it retains royalty) - balance_pre + DEFAULT_NFT_PRICE - platform_fee, + 100u64 - (Content::platform_fee_percentage() * 100u64), ) }) } @@ -508,7 +515,6 @@ fn buy_now_ok_with_nft_owner_curator_group_owned_channel_and_non_set_account_and CreateVideoFixture::default() .with_sender(DEFAULT_CURATOR_ACCOUNT_ID) .with_actor(ContentActor::Curator(group_id, DEFAULT_CURATOR_ID)) - .with_channel_id(NextChannelId::::get() - 1) .call_and_assert(Ok(())); assert_ok!(Content::issue_nft( @@ -609,7 +615,7 @@ pub fn proceeds_are_correctly_credited_if_nft_owned_by_member_channel_with_no_re } #[test] -pub fn nft_channel_member_owner_is_correctly_credited_after_sale() { +pub fn nft_channel_member_owner_with_reward_account_set_is_correctly_credited_after_sale() { with_default_mock_builder(|| { increase_account_balance_helper(SECOND_MEMBER_ACCOUNT_ID, 100u64); CreateChannelFixture::default() From 8d223f3594daea61e53289ee82f1080bbe8c1e9b Mon Sep 17 00:00:00 2001 From: ignazio Date: Thu, 19 May 2022 09:32:06 +0200 Subject: [PATCH 07/17] renaming --- runtime-modules/content/src/lib.rs | 16 ++++++++-------- runtime-modules/content/src/nft/mod.rs | 16 +++++++++------- runtime-modules/content/src/tests/fixtures.rs | 3 ++- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/runtime-modules/content/src/lib.rs b/runtime-modules/content/src/lib.rs index 3bf018f0e2..67b3e5d75a 100644 --- a/runtime-modules/content/src/lib.rs +++ b/runtime-modules/content/src/lib.rs @@ -1275,7 +1275,7 @@ decl_module! { ) -> DispatchResult { let channel = Self::ensure_channel_exists(&item.channel_id)?; - let reward_account = Self::ensure_reward_account(&channel)?; + let reward_account = Self::ensure_channel_has_beneficiary_account(&channel)?; ensure_actor_authorized_to_claim_payment::(origin, &actor, &channel.owner)?; @@ -1662,7 +1662,7 @@ decl_module! { let royalty_payment = Self::build_royalty_payment(&video, nft.creator_royalty); let updated_nft = Self::complete_auction( nft, - video.in_channel, + &video, royalty_payment, participant_id, buy_now_price, @@ -1770,7 +1770,7 @@ decl_module! { let royalty_payment = Self::build_royalty_payment(&video, nft.creator_royalty); let updated_nft = Self::complete_auction( nft, - video.in_channel, + &video, royalty_payment, participant_id, buy_now_price, @@ -1882,7 +1882,7 @@ decl_module! { let royalty_payment = Self::build_royalty_payment(&video, nft.creator_royalty); let updated_nft = Self::complete_auction( nft, - video.in_channel, + &video, royalty_payment, top_bidder_id, top_bid.amount @@ -1935,7 +1935,7 @@ decl_module! { let royalty_payment = Self::build_royalty_payment(&video, nft.creator_royalty); let updated_nft = Self::complete_auction( nft, - video.in_channel, + &video, royalty_payment, winner_id, bid.amount, @@ -2046,7 +2046,7 @@ decl_module! { Self::ensure_new_pending_offer_available_to_proceed(&nft, &receiver_account_id)?; // account_id where the nft offer price is deposited - let nft_owner_account = Self::ensure_nft_owner_account_id(video.in_channel, &nft).ok(); + let nft_owner_account = Self::ensure_nft_owner_has_beneficiary_account(&video, &nft).ok(); // // == MUTATION SAFE == // @@ -2122,7 +2122,7 @@ decl_module! { Self::ensure_can_buy_now(&nft, &participant_account_id, price_commit)?; // seller account - let old_nft_owner_account_id = Self::ensure_nft_owner_account_id(video.in_channel, &nft).ok(); + let old_nft_owner_account_id = Self::ensure_nft_owner_has_beneficiary_account(&video, &nft).ok(); // // == MUTATION SAFE == @@ -2428,7 +2428,7 @@ impl Module { Ok(()) } - pub(crate) fn ensure_reward_account( + pub(crate) fn ensure_channel_has_beneficiary_account( channel: &Channel, ) -> Result { if let Some(reward_account) = &channel.reward_account { diff --git a/runtime-modules/content/src/nft/mod.rs b/runtime-modules/content/src/nft/mod.rs index 8d8f0a5e52..bf115495bc 100644 --- a/runtime-modules/content/src/nft/mod.rs +++ b/runtime-modules/content/src/nft/mod.rs @@ -363,12 +363,12 @@ impl Module { pub(crate) fn complete_auction( nft: Nft, - in_channel: T::ChannelId, + video: &Video, royalty_payment: Option<(Royalty, T::AccountId)>, winner_id: T::MemberId, amount: BalanceOf, ) -> Nft { - let account_deposit_into = Self::ensure_nft_owner_account_id(in_channel, &nft).ok(); + let account_deposit_into = Self::ensure_nft_owner_has_beneficiary_account(video, &nft).ok(); let account_withdraw_from = ContentTreasury::::module_account_id(); Self::complete_payment( @@ -389,15 +389,17 @@ impl Module { /// - `None` -> then if channel owner is: /// - `Member` -> use member controller account /// - `CuratorGroup` -> Error - pub(crate) fn ensure_nft_owner_account_id( - channel_id: T::ChannelId, + /// In order to statically guarantee that `video.in_channel` exists, by leveraging the + /// Runtime invariant: `video` exists => `video.in_channel` exists + pub(crate) fn ensure_nft_owner_has_beneficiary_account( + video: &Video, nft: &Nft, ) -> Result { match nft.owner { NftOwner::Member(member_id) => T::MemberAuthenticator::controller_account_id(member_id), NftOwner::ChannelOwner => { - let channel = Self::ensure_channel_exists(&channel_id)?; - Self::ensure_reward_account(&channel) + let channel = Self::ensure_channel_exists(&video.in_channel)?; + Self::ensure_channel_has_beneficiary_account(&channel) } } } @@ -470,7 +472,7 @@ impl Module { // payment is none if there is no royalty if let Some(royalty) = creator_royalty { let channel = Self::channel_by_id(&video.in_channel); - Self::ensure_reward_account(&channel) + Self::ensure_channel_has_beneficiary_account(&channel) .ok() .map(|reward_acc| (royalty, reward_acc)) } else { diff --git a/runtime-modules/content/src/tests/fixtures.rs b/runtime-modules/content/src/tests/fixtures.rs index c61422c15e..8b2ce2d571 100644 --- a/runtime-modules/content/src/tests/fixtures.rs +++ b/runtime-modules/content/src/tests/fixtures.rs @@ -1374,7 +1374,8 @@ impl ClaimChannelRewardFixture { pub fn call_and_assert(&self, expected_result: DispatchResult) { let origin = Origin::signed(self.sender.clone()); let channel = Content::channel_by_id(self.item.channel_id); - let reward_account = Content::ensure_reward_account(&channel).unwrap_or_default(); + let reward_account = + Content::ensure_channel_has_beneficiary_account(&channel).unwrap_or_default(); let balance_pre = Balances::::usable_balance(&reward_account); let payout_earned_pre = Content::channel_by_id(self.item.channel_id).cumulative_payout_earned; From c77762050b08ae7391f2ee71894eeb869ddd7adf Mon Sep 17 00:00:00 2001 From: ignazio Date: Thu, 19 May 2022 09:34:56 +0200 Subject: [PATCH 08/17] using unchecked channel_by_id --- runtime-modules/content/src/nft/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime-modules/content/src/nft/mod.rs b/runtime-modules/content/src/nft/mod.rs index bf115495bc..a49f3ac88b 100644 --- a/runtime-modules/content/src/nft/mod.rs +++ b/runtime-modules/content/src/nft/mod.rs @@ -398,7 +398,7 @@ impl Module { match nft.owner { NftOwner::Member(member_id) => T::MemberAuthenticator::controller_account_id(member_id), NftOwner::ChannelOwner => { - let channel = Self::ensure_channel_exists(&video.in_channel)?; + let channel = Self::channel_by_id(&video.in_channel); Self::ensure_channel_has_beneficiary_account(&channel) } } @@ -416,7 +416,7 @@ impl Module { ) -> Result, DispatchError> { if let TransactionalStatus::::EnglishAuction(auction) = &nft.transactional_status { Ok(auction.to_owned()) - } else { + } else { Err(Error::::IsNotEnglishAuctionType.into()) } } From c7f95544b0b393080046d6a48c8d20bb47db5fb1 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 19 May 2022 12:38:31 +0400 Subject: [PATCH 09/17] bump spec runtime to 7 --- Cargo.lock | 6 +++--- node/Cargo.toml | 2 +- runtime-modules/content/src/nft/mod.rs | 2 +- runtime/Cargo.toml | 2 +- runtime/src/lib.rs | 2 +- utils/chain-spec-builder/Cargo.toml | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 232225bd6c..16502185e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -722,7 +722,7 @@ dependencies = [ [[package]] name = "chain-spec-builder" -version = "6.6.0" +version = "6.7.0" dependencies = [ "ansi_term 0.12.1", "enum-utils", @@ -2278,7 +2278,7 @@ dependencies = [ [[package]] name = "joystream-node" -version = "6.6.0" +version = "6.7.0" dependencies = [ "frame-benchmarking", "frame-benchmarking-cli", @@ -2339,7 +2339,7 @@ dependencies = [ [[package]] name = "joystream-node-runtime" -version = "10.6.0" +version = "10.7.0" dependencies = [ "frame-benchmarking", "frame-executive", diff --git a/node/Cargo.toml b/node/Cargo.toml index ed47a5c92d..69067b456d 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -3,7 +3,7 @@ authors = ['Joystream contributors'] build = 'build.rs' edition = '2018' name = 'joystream-node' -version = '6.6.0' +version = '6.7.0' default-run = "joystream-node" [[bin]] diff --git a/runtime-modules/content/src/nft/mod.rs b/runtime-modules/content/src/nft/mod.rs index a49f3ac88b..3ededcc2ec 100644 --- a/runtime-modules/content/src/nft/mod.rs +++ b/runtime-modules/content/src/nft/mod.rs @@ -416,7 +416,7 @@ impl Module { ) -> Result, DispatchError> { if let TransactionalStatus::::EnglishAuction(auction) = &nft.transactional_status { Ok(auction.to_owned()) - } else { + } else { Err(Error::::IsNotEnglishAuctionType.into()) } } diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 5fa87a4271..9df2c68c81 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -4,7 +4,7 @@ edition = '2018' name = 'joystream-node-runtime' # Follow convention: https://github.com/Joystream/substrate-runtime-joystream/issues/1 # {Authoring}.{Spec}.{Impl} of the RuntimeVersion -version = '10.6.0' +version = '10.7.0' [dependencies] # Third-party dependencies diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index b7049f957b..fa37e4308a 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -98,7 +98,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("joystream-node"), impl_name: create_runtime_str!("joystream-node"), authoring_version: 10, - spec_version: 6, + spec_version: 7, impl_version: 0, apis: crate::runtime_api::EXPORTED_RUNTIME_API_VERSIONS, transaction_version: 1, diff --git a/utils/chain-spec-builder/Cargo.toml b/utils/chain-spec-builder/Cargo.toml index a4b39ef297..63948a3a07 100644 --- a/utils/chain-spec-builder/Cargo.toml +++ b/utils/chain-spec-builder/Cargo.toml @@ -3,7 +3,7 @@ authors = ['Joystream contributors'] build = 'build.rs' edition = '2018' name = 'chain-spec-builder' -version = '6.6.0' +version = '6.7.0' [dependencies] enum-utils = "0.1.2" From cecad5658117b29a334549300f83a0ad98691d9a Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 19 May 2022 12:40:27 +0400 Subject: [PATCH 10/17] remove setting NFT values in on_runtime_upgrade --- runtime/src/runtime_api.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/runtime/src/runtime_api.rs b/runtime/src/runtime_api.rs index 078d0c9608..cbec572f4e 100644 --- a/runtime/src/runtime_api.rs +++ b/runtime/src/runtime_api.rs @@ -83,9 +83,6 @@ pub struct CustomOnRuntimeUpgrade; impl OnRuntimeUpgrade for CustomOnRuntimeUpgrade { fn on_runtime_upgrade() -> Weight { ProposalsEngine::cancel_active_and_pending_proposals(); - // Set NFT values - >::put(Balance::from(1_000_000_000_000u64)); - >::put(Balance::from(1_000_000_000_000u64)); 10_000_000 // TODO: adjust weight } From ebbc7ba38e30f640c5b89e5b62e8e7d632797185 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 19 May 2022 12:41:00 +0400 Subject: [PATCH 11/17] update runtime-update ci check workflow --- .github/workflows/runtime-upgrade.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/runtime-upgrade.yml b/.github/workflows/runtime-upgrade.yml index f3996ebabf..d9761e6b31 100644 --- a/.github/workflows/runtime-upgrade.yml +++ b/.github/workflows/runtime-upgrade.yml @@ -102,7 +102,7 @@ jobs: export HOME=${PWD} mkdir -p ${HOME}/.local/share/joystream-cli yarn joystream-cli api:setUri ws://localhost:9944 - # Olympia release production runtime profile - export RUNTIME_TAG=6740a4ae2bf40fe7c670fb49943cbbe290277601 + # Rhodes release (spec 6) production runtime profile + export RUNTIME_TAG=8c2e70abcb34a6892991355b61e804b7c1492290 export TARGET_RUNTIME_TAG=latest tests/network-tests/run-migration-tests.sh \ No newline at end of file From 189d4f9fb75f9a12f4e6a07d235d7ce590bb6134 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 19 May 2022 12:45:54 +0400 Subject: [PATCH 12/17] basic rhodes update-1 ci checks --- .../src/misc/assertPostUpgradeConsts.ts | 65 ------------------- .../src/misc/postRuntimUpdateChecks.ts | 14 ++++ .../src/scenarios/postRuntimeUpdate.ts | 4 +- 3 files changed, 16 insertions(+), 67 deletions(-) delete mode 100644 tests/network-tests/src/misc/assertPostUpgradeConsts.ts create mode 100644 tests/network-tests/src/misc/postRuntimUpdateChecks.ts diff --git a/tests/network-tests/src/misc/assertPostUpgradeConsts.ts b/tests/network-tests/src/misc/assertPostUpgradeConsts.ts deleted file mode 100644 index 919b18a735..0000000000 --- a/tests/network-tests/src/misc/assertPostUpgradeConsts.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { assert } from 'chai' -import { FlowProps } from '../Flow' -import { extendDebug } from '../Debugger' -import { CreateProposalsFixture } from '../fixtures/proposals/CreateProposalsFixture' -import { BuyMembershipHappyCaseFixture } from '../fixtures/membership/BuyMembershipHappyCaseFixture' -import { FixtureRunner } from '../Fixture' -import BN from 'bn.js' - -export default async function assertValues({ api, query }: FlowProps): Promise { - const debug = extendDebug('flow:postMigrationAssertions') - debug('Started') - - debug('Check runtime spec version') - const version = await api.rpc.state.getRuntimeVersion() - assert.equal(version.specVersion.toNumber(), 6) - - debug('Check that post migration NFT value are updated') - - const maxNftStartingPrice = (await api.query.content.maxStartingPrice()).toNumber() - const maxNftBidStep = (await api.query.content.maxBidStep()).toNumber() - - // These values are expected on production runtime profile - assert.equal(maxNftStartingPrice, 1000000000000) - assert.equal(maxNftBidStep, 1000000000000) - - debug('Check that post migration Forum values are updated') - - const maxForumCategories = api.consts.forum.maxCategories.toNumber() - const maxForumSubCategories = api.consts.forum.maxSubcategories.toNumber() - - assert.equal(maxForumCategories, 40) - assert.equal(maxForumSubCategories, 40) - - debug('Check set_council_budget_increment_proposal grace period') - - // Grant treasury account large balance to transfer to new member for buying membership - // and staking. From a forked state there is no guarantee the account has sufficient funds. - await api.grantTreasuryWorkingBalance() - - debug('creating new member') - const [memberControllerAcc] = (await api.createKeyPairs(1)).map(({ key }) => key.address) - const buyMembershipFixture = new BuyMembershipHappyCaseFixture(api, query, [memberControllerAcc]) - await new FixtureRunner(buyMembershipFixture).run() - const [memberId] = buyMembershipFixture.getCreatedMembers() - - debug('creating proposal') - const createProposal = new CreateProposalsFixture(api, query, [ - { - asMember: memberId, - title: 'test proposal', - description: 'testing council budget increment proposal grace period', - exactExecutionBlock: undefined, - type: 'SetCouncilBudgetIncrement', - details: 1_000_000, - }, - ]) - - await new FixtureRunner(createProposal).run() - debug('checking proposal parameters') - const proposalId = createProposal.getCreatedProposalsIds()[0] - const proposal = await api.query.proposalsEngine.proposals(proposalId) - assert.equal(proposal.parameters.gracePeriod.toNumber(), 14400) - - debug('Done') -} diff --git a/tests/network-tests/src/misc/postRuntimUpdateChecks.ts b/tests/network-tests/src/misc/postRuntimUpdateChecks.ts new file mode 100644 index 0000000000..2d8c6c2d07 --- /dev/null +++ b/tests/network-tests/src/misc/postRuntimUpdateChecks.ts @@ -0,0 +1,14 @@ +import { assert } from 'chai' +import { FlowProps } from '../Flow' +import { extendDebug } from '../Debugger' + +export default async function assertValues({ api }: FlowProps): Promise { + const debug = extendDebug('flow:postMigrationAssertions') + debug('Started') + + debug('Check runtime spec version') + const version = await api.rpc.state.getRuntimeVersion() + assert.equal(version.specVersion.toNumber(), 7) + + debug('Done') +} diff --git a/tests/network-tests/src/scenarios/postRuntimeUpdate.ts b/tests/network-tests/src/scenarios/postRuntimeUpdate.ts index 96043327b9..e449553848 100644 --- a/tests/network-tests/src/scenarios/postRuntimeUpdate.ts +++ b/tests/network-tests/src/scenarios/postRuntimeUpdate.ts @@ -1,7 +1,7 @@ import { scenario } from '../Scenario' -import assertValues from '../misc/assertPostUpgradeConsts' +import postRuntimeUpdateChecks from '../misc/postRuntimUpdateChecks' scenario('Post Runtime Upgrade', async ({ job }) => { // Verify constants - job('Verify Values', assertValues) + job('Run Checks', postRuntimeUpdateChecks) }) From 5d5e80e33c0cbc786b468cd45c1aa4f3e5fd25a1 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 19 May 2022 12:50:17 +0400 Subject: [PATCH 13/17] clippy fix --- runtime/src/runtime_api.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/runtime/src/runtime_api.rs b/runtime/src/runtime_api.rs index cbec572f4e..56682bef0b 100644 --- a/runtime/src/runtime_api.rs +++ b/runtime/src/runtime_api.rs @@ -1,5 +1,4 @@ use frame_support::inherent::{CheckInherentsResult, InherentData}; -use frame_support::storage::StorageValue; use frame_support::traits::{KeyOwnerProofSystem, OnRuntimeUpgrade, Randomness}; use frame_support::unsigned::{TransactionSource, TransactionValidity}; use pallet_grandpa::fg_primitives; From 9751a3ab2c7e28c8d0f54be82055a21a090e2b5f Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 19 May 2022 13:21:32 +0400 Subject: [PATCH 14/17] appropriate name for runtime update job in ci check --- .github/workflows/runtime-upgrade.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/runtime-upgrade.yml b/.github/workflows/runtime-upgrade.yml index d9761e6b31..863fa8a2a7 100644 --- a/.github/workflows/runtime-upgrade.yml +++ b/.github/workflows/runtime-upgrade.yml @@ -71,9 +71,9 @@ jobs: name: ${{ steps.compute_shasum.outputs.shasum }}-joystream-node-docker-image.tar.gz path: joystream-node-docker-image.tar.gz - runtime_upgrade_from_olympia: + runtime_upgrade: # if: ${{ false }} - name: Runtime Upgrade From Olypia + name: Runtime Upgrade From Rhodes Spec 6 needs: build_images runs-on: ubuntu-latest steps: From fb2bec1bebe2bd49d250fc9dbb5649b26d76d26f Mon Sep 17 00:00:00 2001 From: ignazio Date: Thu, 19 May 2022 11:35:29 +0200 Subject: [PATCH 15/17] cargo fmt --- runtime-modules/content/src/nft/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime-modules/content/src/nft/mod.rs b/runtime-modules/content/src/nft/mod.rs index a49f3ac88b..3ededcc2ec 100644 --- a/runtime-modules/content/src/nft/mod.rs +++ b/runtime-modules/content/src/nft/mod.rs @@ -416,7 +416,7 @@ impl Module { ) -> Result, DispatchError> { if let TransactionalStatus::::EnglishAuction(auction) = &nft.transactional_status { Ok(auction.to_owned()) - } else { + } else { Err(Error::::IsNotEnglishAuctionType.into()) } } From 3b62f4922aed8c2f256d4577f4323b121c9bd436 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Fri, 20 May 2022 15:47:58 +0400 Subject: [PATCH 16/17] runtime CHANGELOG --- runtime/CHANGELOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/runtime/CHANGELOG.md b/runtime/CHANGELOG.md index 8e480c856c..f8decde76a 100644 --- a/runtime/CHANGELOG.md +++ b/runtime/CHANGELOG.md @@ -1,3 +1,21 @@ +### Version 10.7.0 - upgrade +- NFT channel proceeds bug fix #3763 + Fix logic in dispatch calls: + content::claim_channel_reward() + content::pick_open_auction_winner() +- No runtime types changed + +### Version 10.6.0 - Rhodes - upgrade +- Enable NFT functionality +- Types updated - types pacakge version v0.19.3 + +### Version 10.5.0 - Olympia - new chain +- New feature new Membership system +- New feature Improved Council and Election system +- New feature Bountines +- Forum improvements +- New types package - version v0.18.3 + ### Version 9.14.0 - Giza - upgrade - New storage and distribution runtime module - Renaming of working groups and adding new working group for distributor role From 6710ee11cb63522dd29e24be6ca76b6bd6f52686 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Fri, 20 May 2022 16:16:01 +0400 Subject: [PATCH 17/17] changelog type fix and additional details --- runtime/CHANGELOG.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/runtime/CHANGELOG.md b/runtime/CHANGELOG.md index f8decde76a..162d234e72 100644 --- a/runtime/CHANGELOG.md +++ b/runtime/CHANGELOG.md @@ -1,18 +1,22 @@ ### Version 10.7.0 - upgrade -- NFT channel proceeds bug fix #3763 - Fix logic in dispatch calls: - content::claim_channel_reward() - content::pick_open_auction_winner() +- NFT channel proceeds bug fix [#3763](https://github.com/Joystream/joystream/pull/3763) + - Fix logic in dispatch calls: `content::claim_channel_reward()`, `content::pick_open_auction_winner()` - No runtime types changed ### Version 10.6.0 - Rhodes - upgrade - Enable NFT functionality - Types updated - types pacakge version v0.19.3 +- Modified some runtime constants and initial [values](https://github.com/Joystream/joystream/pull/3678): + - Changed NFT parameters `MaxStartingPrice` and `MaxBidStep` + - Changed inflation curve, to reduce validator rewards + - Changed grace period for proposal types Set council budget increment + - Changed forum `MaxSubcategories` and `MaxCategories` ### Version 10.5.0 - Olympia - new chain - New feature new Membership system - New feature Improved Council and Election system -- New feature Bountines +- New feature Bounties +- New NFT feature - Disabled to simplify next update - Forum improvements - New types package - version v0.18.3