diff --git a/prdoc/pr_11425.prdoc b/prdoc/pr_11425.prdoc new file mode 100644 index 0000000000000..e3387bd826c1c --- /dev/null +++ b/prdoc/pr_11425.prdoc @@ -0,0 +1,12 @@ +title: 'fix(pallet-multi-asset-bounties): use non-destructive read in calculate_payout()' +doc: +- audience: Runtime Dev + description: | + Fix `calculate_payout()` using `ChildBountiesValuePerParent::take()` instead of `get()`. + The destructive `take()` deletes the storage entry on first call, causing + `BountyPayoutProcessed` to emit an incorrect payout value when `check_status()` calls + `calculate_payout()` a second time on the success path. Replaced `take()` with `get()` + and moved storage cleanup to `remove_bounty()`. +crates: +- name: pallet-multi-asset-bounties + bump: patch diff --git a/substrate/frame/multi-asset-bounties/src/lib.rs b/substrate/frame/multi-asset-bounties/src/lib.rs index a5a7a3ffa60ab..408dbc91287c7 100644 --- a/substrate/frame/multi-asset-bounties/src/lib.rs +++ b/substrate/frame/multi-asset-bounties/src/lib.rs @@ -98,7 +98,7 @@ use frame_system::pallet_prelude::{ }; use scale_info::TypeInfo; use sp_runtime::{ - traits::{AccountIdConversion, BadOrigin, Convert, Saturating, StaticLookup, TryConvert, Zero}, + traits::{AccountIdConversion, BadOrigin, Convert, Saturating, StaticLookup, TryConvert}, Permill, RuntimeDebug, }; @@ -1551,7 +1551,7 @@ impl, I: 'static> Pallet { None => { // Get total child bounties value, and subtract it from the parent // value. - let children_value = ChildBountiesValuePerParent::::take(parent_bounty_id); + let children_value = ChildBountiesValuePerParent::::get(parent_bounty_id); debug_assert!(children_value <= value); let payout = value.saturating_sub(children_value); payout @@ -1571,7 +1571,7 @@ impl, I: 'static> Pallet { Bounties::::remove(parent_bounty_id); ChildBountiesPerParent::::remove(parent_bounty_id); TotalChildBountiesPerParent::::remove(parent_bounty_id); - debug_assert!(ChildBountiesValuePerParent::::get(parent_bounty_id).is_zero()); + ChildBountiesValuePerParent::::remove(parent_bounty_id); }, Some(child_bounty_id) => { ChildBounties::::remove(parent_bounty_id, child_bounty_id); diff --git a/substrate/frame/multi-asset-bounties/src/tests.rs b/substrate/frame/multi-asset-bounties/src/tests.rs index 40d39e47a88fc..2bdbb82f1917d 100644 --- a/substrate/frame/multi-asset-bounties/src/tests.rs +++ b/substrate/frame/multi-asset-bounties/src/tests.rs @@ -947,7 +947,16 @@ fn check_status_works() { set_status(payment_id, PaymentStatus::Success); assert_ok!(Bounties::check_status(RuntimeOrigin::signed(1), s.parent_bounty_id, None)); - // Then + // Then: BountyPayoutProcessed should emit the net payout (parent value minus child + // value), not the full parent value. + let expected_payout = s.value - s.child_value; + expect_events(vec![BountiesEvent::BountyPayoutProcessed { + index: s.parent_bounty_id, + child_index: None, + asset_kind: s.asset_kind, + value: expected_payout, + beneficiary: s.beneficiary, + }]); assert_eq!( pallet_bounties::ChildBountiesValuePerParent::::get(s.parent_bounty_id), 0