Skip to content

fix(pallet-multi-asset-bounties): use non-destructive read in calculate_payout()#11425

Merged
dhirajs0 merged 12 commits intomasterfrom
dhiraj-fix-multi-asset-bounties-calculate-payout-use-get
Apr 2, 2026
Merged

fix(pallet-multi-asset-bounties): use non-destructive read in calculate_payout()#11425
dhirajs0 merged 12 commits intomasterfrom
dhiraj-fix-multi-asset-bounties-calculate-payout-use-get

Conversation

@dhirajs0
Copy link
Copy Markdown
Contributor

@dhirajs0 dhirajs0 commented Mar 18, 2026

Description

calculate_payout() in pallet-multi-asset-bounties uses ChildBountiesValuePerParent::take() — a destructive read that deletes the storage entry — instead of get(). Since calculate_payout() is called from multiple code paths, the take() causes incorrect behavior on subsequent calls.

calculate_payout() is called from two places:

  1. do_process_payout_payment() (lib.rs:1736) — invoked by award_bounty() and retry_payment()
  2. do_check_payout_payment_status() (lib.rs:1771) — invoked by check_status() on payment success

When a parent bounty with child bounties is awarded:

  • The first call (from award_bounty()) reads ChildBountiesValuePerParent via take(), correctly computing parent_value - children_value, but deletes the storage entry as a side effect.
  • The second call (from check_status() on success) reads the now-deleted storage, gets 0, and emits BountyPayoutProcessed with value: parent_value instead of the correct value: parent_value - children_value.

Additionally, if a non-synchronous Paymaster implementation is used where check_payment() can return Failure, the retry_payment() path would call calculate_payout() again on the deleted storage, attempting to pay the full parent value instead of the reduced amount.

Integration

No integration changes required for downstream projects. This is a bugfix internal to pallet-multi-asset-bounties with no changes to public APIs, storage layout, or trait definitions.

Review Notes

Three changes were made:

1. calculate_payout()take() replaced with get()

- let children_value = ChildBountiesValuePerParent::<T, I>::take(parent_bounty_id);
+ let children_value = ChildBountiesValuePerParent::<T, I>::get(parent_bounty_id);

This makes calculate_payout() idempotent — safe to call multiple times for the same bounty.

2. remove_bounty() — explicit storage cleanup added

  None => {
      Bounties::<T, I>::remove(parent_bounty_id);
      ChildBountiesPerParent::<T, I>::remove(parent_bounty_id);
      TotalChildBountiesPerParent::<T, I>::remove(parent_bounty_id);
-     debug_assert!(ChildBountiesValuePerParent::<T, I>::get(parent_bounty_id).is_zero());
+     ChildBountiesValuePerParent::<T, I>::remove(parent_bounty_id);
  },

The debug_assert! was removed because it was not a true invariant — it only passed because take() had already deleted the value. When child bounties are paid out, ChildBountiesValuePerParent remains non-zero until parent bounty cleanup.

3. Test updated

Added an event assertion to the existing check_status_works test to verify BountyPayoutProcessed emits the correct net payout value (parent_value - child_value) instead of the full parent value. This assertion fails with take() and passes with get().

Impact

  • Current deployments (KAH, PAH): Both use LocalPay where check_payment() always returns Success. The retry/lock path is unreachable, but the BountyPayoutProcessed event emits an incorrect payout value for parent bounties with child bounties.
  • Future deployments: If the pallet is configured with an async Paymaster (e.g., XCM-based) where check_payment() can return Failure, the retry_payment() path would compute a wrong payout amount, potentially leading to permanent fund lock with no recovery path (since close_bounty() rejects PayoutAttempted status).

@dhirajs0
Copy link
Copy Markdown
Contributor Author

/cmd fmt

@dhirajs0
Copy link
Copy Markdown
Contributor Author

/cmd prdoc

@dhirajs0 dhirajs0 requested a review from muharem March 18, 2026 19:12
@dhirajs0 dhirajs0 added T1-FRAME This PR/Issue is related to core FRAME, the framework. A4-backport-stable2506 Pull request must be backported to the stable2506 release branch A4-backport-stable2509 Pull request must be backported to the stable2509 release branch A4-backport-stable2512 Pull request must be backported to the stable2512 release branch A4-backport-stable2603 Pull request must be backported to the stable2603 release branch labels Mar 19, 2026
@dhirajs0 dhirajs0 added A4-backport-stable2509 Pull request must be backported to the stable2509 release branch and removed A4-backport-stable2506 Pull request must be backported to the stable2506 release branch A4-backport-stable2509 Pull request must be backported to the stable2509 release branch labels Mar 23, 2026
Comment thread substrate/frame/multi-asset-bounties/src/lib.rs
@dhirajs0 dhirajs0 enabled auto-merge March 25, 2026 14:58
@Szegoo Szegoo self-requested a review March 31, 2026 15:59
@github-actions
Copy link
Copy Markdown
Contributor

Review required! Latest push from author must always be reviewed

@muharem muharem self-requested a review March 31, 2026 16:00
@dhirajs0 dhirajs0 requested review from BigTava and ggwpez April 1, 2026 15:16
@Szegoo Szegoo self-requested a review April 2, 2026 12:04
@dhirajs0 dhirajs0 added this pull request to the merge queue Apr 2, 2026
Merged via the queue into master with commit 5fbeadd Apr 2, 2026
255 of 260 checks passed
@dhirajs0 dhirajs0 deleted the dhiraj-fix-multi-asset-bounties-calculate-payout-use-get branch April 2, 2026 12:53
@paritytech-release-backport-bot
Copy link
Copy Markdown

Created backport PR for stable2512:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-11425-to-stable2512
git worktree add --checkout .worktree/backport-11425-to-stable2512 backport-11425-to-stable2512
cd .worktree/backport-11425-to-stable2512
git reset --hard HEAD^
git cherry-pick -x 5fbeadde2a12aa6c3bd97b64a82748e715ae7813
git push --force-with-lease

paritytech-release-backport-bot Bot pushed a commit that referenced this pull request Apr 2, 2026
…te_payout() (#11425)

## Description

`calculate_payout()` in `pallet-multi-asset-bounties` uses
`ChildBountiesValuePerParent::take()` — a destructive read that deletes
the storage entry — instead of `get()`. Since `calculate_payout()` is
called from multiple code paths, the `take()` causes incorrect behavior
on subsequent calls.

`calculate_payout()` is called from two places:

1. `do_process_payout_payment()` (lib.rs:1736) — invoked by
`award_bounty()` and `retry_payment()`
2. `do_check_payout_payment_status()` (lib.rs:1771) — invoked by
`check_status()` on payment success

When a parent bounty with child bounties is awarded:

- The **first call** (from `award_bounty()`) reads
`ChildBountiesValuePerParent` via `take()`, correctly computing
`parent_value - children_value`, but **deletes the storage entry** as a
side effect.
- The **second call** (from `check_status()` on success) reads the
now-deleted storage, gets `0`, and emits `BountyPayoutProcessed` with
`value: parent_value` instead of the correct `value: parent_value -
children_value`.

Additionally, if a non-synchronous `Paymaster` implementation is used
where `check_payment()` can return `Failure`, the `retry_payment()` path
would call `calculate_payout()` again on the deleted storage, attempting
to pay the full parent value instead of the reduced amount.

## Integration

No integration changes required for downstream projects. This is a
bugfix internal to `pallet-multi-asset-bounties` with no changes to
public APIs, storage layout, or trait definitions.

## Review Notes

Three changes were made:

### 1. `calculate_payout()` — `take()` replaced with `get()`

```diff
- let children_value = ChildBountiesValuePerParent::<T, I>::take(parent_bounty_id);
+ let children_value = ChildBountiesValuePerParent::<T, I>::get(parent_bounty_id);
```

This makes `calculate_payout()` idempotent — safe to call multiple times
for the same bounty.

### 2. `remove_bounty()` — explicit storage cleanup added

```diff
  None => {
      Bounties::<T, I>::remove(parent_bounty_id);
      ChildBountiesPerParent::<T, I>::remove(parent_bounty_id);
      TotalChildBountiesPerParent::<T, I>::remove(parent_bounty_id);
-     debug_assert!(ChildBountiesValuePerParent::<T, I>::get(parent_bounty_id).is_zero());
+     ChildBountiesValuePerParent::<T, I>::remove(parent_bounty_id);
  },
```

The `debug_assert!` was removed because it was not a true invariant — it
only passed because `take()` had already deleted the value. When child
bounties are paid out, `ChildBountiesValuePerParent` remains non-zero
until parent bounty cleanup.

### 3. Test updated

Added an event assertion to the existing `check_status_works` test to
verify `BountyPayoutProcessed` emits the correct net payout value
(`parent_value - child_value`) instead of the full parent value. This
assertion fails with `take()` and passes with `get()`.

### Impact

- **Current deployments (KAH, PAH)**: Both use `LocalPay` where
`check_payment()` always returns `Success`. The retry/lock path is
unreachable, but the `BountyPayoutProcessed` event emits an incorrect
payout value for parent bounties with child bounties.
- **Future deployments**: If the pallet is configured with an async
`Paymaster` (e.g., XCM-based) where `check_payment()` can return
`Failure`, the `retry_payment()` path would compute a wrong payout
amount, potentially leading to permanent fund lock with no recovery path
(since `close_bounty()` rejects `PayoutAttempted` status).

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 5fbeadd)
@paritytech-release-backport-bot
Copy link
Copy Markdown

Successfully created backport PR for stable2603:

lexnv added a commit that referenced this pull request Apr 3, 2026
Squashed commit of the following:

commit e9ab097
Author: eskimor <robert@gonimo.com>
Date:   Fri Apr 3 00:53:19 2026 +0200

    Pr doc

commit ef9dfac
Author: eskimor <robert@gonimo.com>
Date:   Fri Apr 3 00:41:08 2026 +0200

    Fix warning

commit 076e7e9
Author: eskimor <robert@gonimo.com>
Date:   Fri Apr 3 00:09:28 2026 +0200

    Tests

commit cdfc57c
Author: eskimor <robert@gonimo.com>
Date:   Fri Apr 3 00:09:03 2026 +0200

    Fix test

commit a1a2bbf
Author: clangenb <37865735+clangenb@users.noreply.github.com>
Date:   Thu Apr 2 22:55:34 2026 +0200

    Fix slot-based collator panic during warp sync (#11072) (#11381)

    When a parachain collator starts with `--authoring=slot-based` and
    performs warp sync, the `slot-based-block-builder` essential task
    immediately calls `slot_duration()` which requires
    `AuraApi_slot_duration`. During warp sync the runtime isn't ready, so
    this fails and the task returns, shutting down the node.

    The lookahead collator avoids this by calling `wait_for_aura()` before
    starting. This PR adds an equivalent guard to the slot-based collator.

    ### Manual test
    Before the fix the collator panicked after the relay chain warp sync
    with AuraApi_slot_duration not available, which does not occur anymore
    now.
    ```
     ./target/release/polkadot-parachain \
        --chain asset-hub-polkadot \
        --sync warp \
        --authoring=slot-based \
        --tmp -- --sync warp
    ```
    Closes #11072.

    ---------

    Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
    Co-authored-by: clangenb <clangenb@users.noreply.github.com>

commit 802db0b
Author: 0xRVE <robertvaneerdewijk@gmail.com>
Date:   Thu Apr 2 16:39:00 2026 +0300

    [pallet-revive] Add vesting precompile (#11398)

    ## Summary

    Adds a new built-in precompile (`pallet-revive-precompile-vesting`) that
    exposes Substrate's `pallet-vesting` to EVM contracts via pallet-revive.
    EVM contracts can call `vest()`, `vestOther(address)`,
    `vestingBalance()`, and `vestingBalanceOf(address)` at the precompile
    address `0x0902`.

    ## Changes

    - **`substrate/frame/revive/uapi/sol/IVesting.sol`**: New Solidity
    interface defining the vesting precompile ABI
    - **`substrate/frame/revive/uapi/src/precompiles/vesting.rs`**: Binds
    the Solidity interface via `alloy_core::sol!`
    - **`substrate/frame/revive/precompiles/`**: New crate implementing the
    `Precompile` trait — dispatches `vest`/`vestOther` through
    `pallet_vesting` and queries locked balances via `VestingSchedule`
    - **`substrate/frame/revive/src/tests.rs`**: Trailing comma fix in
    `construct_runtime!`

    ## Test plan
    - [x] New vesting precompile tests pass (`vest`, `vestOther`,
    `vestingBalance`, `vestingBalanceOf`)
    - [x] Existing pallet-revive tests unaffected

    ---------

    Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
    Co-authored-by: PG Herveou <pgherveou@gmail.com>

commit 5fbeadd
Author: Dhiraj Sah <dhiraj@parity.io>
Date:   Thu Apr 2 17:35:52 2026 +0530

    fix(pallet-multi-asset-bounties): use non-destructive read in calculate_payout() (#11425)

    ## Description

    `calculate_payout()` in `pallet-multi-asset-bounties` uses
    `ChildBountiesValuePerParent::take()` — a destructive read that deletes
    the storage entry — instead of `get()`. Since `calculate_payout()` is
    called from multiple code paths, the `take()` causes incorrect behavior
    on subsequent calls.

    `calculate_payout()` is called from two places:

    1. `do_process_payout_payment()` (lib.rs:1736) — invoked by
    `award_bounty()` and `retry_payment()`
    2. `do_check_payout_payment_status()` (lib.rs:1771) — invoked by
    `check_status()` on payment success

    When a parent bounty with child bounties is awarded:

    - The **first call** (from `award_bounty()`) reads
    `ChildBountiesValuePerParent` via `take()`, correctly computing
    `parent_value - children_value`, but **deletes the storage entry** as a
    side effect.
    - The **second call** (from `check_status()` on success) reads the
    now-deleted storage, gets `0`, and emits `BountyPayoutProcessed` with
    `value: parent_value` instead of the correct `value: parent_value -
    children_value`.

    Additionally, if a non-synchronous `Paymaster` implementation is used
    where `check_payment()` can return `Failure`, the `retry_payment()` path
    would call `calculate_payout()` again on the deleted storage, attempting
    to pay the full parent value instead of the reduced amount.

    ## Integration

    No integration changes required for downstream projects. This is a
    bugfix internal to `pallet-multi-asset-bounties` with no changes to
    public APIs, storage layout, or trait definitions.

    ## Review Notes

    Three changes were made:

    ### 1. `calculate_payout()` — `take()` replaced with `get()`

    ```diff
    - let children_value = ChildBountiesValuePerParent::<T, I>::take(parent_bounty_id);
    + let children_value = ChildBountiesValuePerParent::<T, I>::get(parent_bounty_id);
    ```

    This makes `calculate_payout()` idempotent — safe to call multiple times
    for the same bounty.

    ### 2. `remove_bounty()` — explicit storage cleanup added

    ```diff
      None => {
          Bounties::<T, I>::remove(parent_bounty_id);
          ChildBountiesPerParent::<T, I>::remove(parent_bounty_id);
          TotalChildBountiesPerParent::<T, I>::remove(parent_bounty_id);
    -     debug_assert!(ChildBountiesValuePerParent::<T, I>::get(parent_bounty_id).is_zero());
    +     ChildBountiesValuePerParent::<T, I>::remove(parent_bounty_id);
      },
    ```

    The `debug_assert!` was removed because it was not a true invariant — it
    only passed because `take()` had already deleted the value. When child
    bounties are paid out, `ChildBountiesValuePerParent` remains non-zero
    until parent bounty cleanup.

    ### 3. Test updated

    Added an event assertion to the existing `check_status_works` test to
    verify `BountyPayoutProcessed` emits the correct net payout value
    (`parent_value - child_value`) instead of the full parent value. This
    assertion fails with `take()` and passes with `get()`.

    ### Impact

    - **Current deployments (KAH, PAH)**: Both use `LocalPay` where
    `check_payment()` always returns `Success`. The retry/lock path is
    unreachable, but the `BountyPayoutProcessed` event emits an incorrect
    payout value for parent bounties with child bounties.
    - **Future deployments**: If the pallet is configured with an async
    `Paymaster` (e.g., XCM-based) where `check_payment()` can return
    `Failure`, the `retry_payment()` path would compute a wrong payout
    amount, potentially leading to permanent fund lock with no recovery path
    (since `close_bounty()` rejects `PayoutAttempted` status).

    ---------

    Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

commit e3865cf
Author: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com>
Date:   Thu Apr 2 10:41:38 2026 +0100

    Deprecate `ValidateUnsigned` trait and `#[pallet::validate_unsigned]` attribute (#10150)

    Part of #2415
    Closes #2436

    Related: #6325 #6326

    ## Summary

    Deprecates the `ValidateUnsigned` trait and
    `#[pallet::validate_unsigned]` attribute in favor of the new
    `TransactionExtension` API. This is a non-breaking change that adds
    deprecation warnings to guide users toward the modern transaction
    validation approach.

    ## Motivation

    The `ValidateUnsigned` trait was the legacy approach for validating
    unsigned transactions in FRAME pallets. The newer `TransactionExtension`
    trait provides a more flexible and composable way to handle transaction
    validation, including both signed and unsigned transactions.

    ## Changes

    ### Deprecated APIs
    - ✅ Added `#[deprecated]` attribute to `ValidateUnsigned` trait
    - ✅ Added deprecation warning to `#[pallet::validate_unsigned]` macro
    attribute

    ### Migration (Using `TransactionExtensions`)

    https://paritytech.github.io/polkadot-sdk/master/polkadot_sdk_docs/reference_docs/transaction_extensions

    ## Impact

    - **Non-breaking:** Existing code continues to work with deprecation
    warnings
    - **Compiler warnings:** Users will see deprecation notices guiding them
    to migrate
    - **Timeline:** Full removal planned for a future major release (TBD)

    ## Review Notes

    - The `#[pallet::validate_unsigned]` deprecation warning might be
    redundant since it's always used together with `ValidateUnsigned`, but
    both are included for completeness and clarity.

    ## Follow-up Tasks

    The following pallets and crates need to be migrated to
    `TransactionExtension` in subsequent PRs:

    **Runtime crates:**
    - [ ] `polkadot-runtime-common`
    - [ ] `polkadot-runtime-parachains`

    **FRAME pallets:**
    - [ ] `pallet-babe`
    - [ ] `pallet-beefy`
    - [ ] `pallet-election-provider-multi-block`
    - [ ] `pallet-grandpa`
    - [x] `pallet-im-online`
    #11235
    - [x] `pallet-mixnet`
    #11010

    **Core:**
    - [ ] `frame-executive`
    - [ ] `frame-system`

    **Examples:**
    - [x] `pallet-example-offchain-worker`
    #10716

    **Testing:**
    - [ ] `substrate-test-runtime`

    ## Open Question

    Should we remove the `ValidateUnsigned` bound from the type parameter
    `V` in the `Applyable` trait?

    ---------

    Co-authored-by: Guillaume Thiolliere <guillaume.thiolliere@parity.io>
    Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
    Co-authored-by: Branislav Kontur <bkontur@gmail.com>

commit 5c97c0f
Author: clangenb <37865735+clangenb@users.noreply.github.com>
Date:   Thu Apr 2 01:06:59 2026 +0200

    [Penpal] fix genesis presets - assign proper ED to accounts (#11575)

    Penpal had values below the ED for initializing asset balances for some
    accounts. This has not been detected as no unit tests actually use the
    presets. This PR fixes the invalid values, and it also adds some unit
    tests for validating that the presets build at least.

    Closes #11558.

    ---------

    Co-authored-by: clangenb <clangenb@users.noreply.github.com>
    Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

commit 59c4053
Author: Egor_P <egor@parity.io>
Date:   Wed Apr 1 20:29:31 2026 +0200

    [Release|CI/CD] Fixes for release flows (#11578)

    This PR backports few fixes for soem release flows, that were made in
    stable2603 branch. In particular:
    - Fixed resume check in Crates Publish flow
    - Fixed missing llvm path on macos builds
    - Fixed scrtipt that reverts path deps in Cargo.toml files
    - Bumped parity-publish version
    - Fixed check if the post-crates-release branch exists in Crateds
    Publish flow

    ---------

    Co-authored-by: BDevParity <bruno.devic@parity.io>

commit e9e4769
Author: DenzelPenzel <15388928+DenzelPenzel@users.noreply.github.com>
Date:   Wed Apr 1 14:11:28 2026 +0100

    Add statement store e2e integration tests (#11237)

    # Description

    #10783

    E2E Integration Tests (zombienet-sdk)

    Functional tests (statement_store)
    - statement_store_genesis_inject — submit + subscribe round-trip with
    genesis-injected allowances, 2-node propagation with data
    integrity verification
    - statement_store_sudo_allowance — sudo-based runtime allowance setting,
    8 concurrent multi-account submissions, 4-node fan-out propagation

    ### Test Infrastructure
    - common.rs — shared helpers: create_test_statement, submit_statement,
    expect_one_statement, expect_statements_unordered,
    subscribe_topic, spawn_network, spawn_network_sudo
    - sc_statement_store::subxt_client — custom subxt config (CustomConfig)
    for non-standard transaction extensions
    (VerifyMultiSignature, RestrictOrigins), set_allowances_via_sudo for
    runtime-configured networks
    - sc_statement_store::test_utils — shared keypair generation and
    allowance storage item builders (get_keypair,
    create_allowance_items, create_uniform_allowance_items)
    - CI matrix registration for all statement store test groups

    ### What we cover in this PR

    Test(statement_store_sudo_allowance) cover the next options for #11534 :
    - Propagation under normal load: Zombienet tests to cover concurrent
    multi-client corner cases and verify statements reach all real nodes,
    NOT including during major sync

    ---------

    Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

commit a41bb09
Author: Javier Viola <363911+pepoviola@users.noreply.github.com>
Date:   Wed Apr 1 14:34:27 2026 +0200

    [zombienet] typo in preflight (#11602)

    Typo in the preflight logic was merged by accident.
    Thx!

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
dhirajs0 added a commit that referenced this pull request Apr 7, 2026
…te_payout() (#11425)

`calculate_payout()` in `pallet-multi-asset-bounties` uses
`ChildBountiesValuePerParent::take()` — a destructive read that deletes
the storage entry — instead of `get()`. Since `calculate_payout()` is
called from multiple code paths, the `take()` causes incorrect behavior
on subsequent calls.

`calculate_payout()` is called from two places:

1. `do_process_payout_payment()` (lib.rs:1736) — invoked by
`award_bounty()` and `retry_payment()`
2. `do_check_payout_payment_status()` (lib.rs:1771) — invoked by
`check_status()` on payment success

When a parent bounty with child bounties is awarded:

- The **first call** (from `award_bounty()`) reads
`ChildBountiesValuePerParent` via `take()`, correctly computing
`parent_value - children_value`, but **deletes the storage entry** as a
side effect.
- The **second call** (from `check_status()` on success) reads the
now-deleted storage, gets `0`, and emits `BountyPayoutProcessed` with
`value: parent_value` instead of the correct `value: parent_value -
children_value`.

Additionally, if a non-synchronous `Paymaster` implementation is used
where `check_payment()` can return `Failure`, the `retry_payment()` path
would call `calculate_payout()` again on the deleted storage, attempting
to pay the full parent value instead of the reduced amount.

No integration changes required for downstream projects. This is a
bugfix internal to `pallet-multi-asset-bounties` with no changes to
public APIs, storage layout, or trait definitions.

Three changes were made:

```diff
- let children_value = ChildBountiesValuePerParent::<T, I>::take(parent_bounty_id);
+ let children_value = ChildBountiesValuePerParent::<T, I>::get(parent_bounty_id);
```

This makes `calculate_payout()` idempotent — safe to call multiple times
for the same bounty.

```diff
  None => {
      Bounties::<T, I>::remove(parent_bounty_id);
      ChildBountiesPerParent::<T, I>::remove(parent_bounty_id);
      TotalChildBountiesPerParent::<T, I>::remove(parent_bounty_id);
-     debug_assert!(ChildBountiesValuePerParent::<T, I>::get(parent_bounty_id).is_zero());
+     ChildBountiesValuePerParent::<T, I>::remove(parent_bounty_id);
  },
```

The `debug_assert!` was removed because it was not a true invariant — it
only passed because `take()` had already deleted the value. When child
bounties are paid out, `ChildBountiesValuePerParent` remains non-zero
until parent bounty cleanup.

Added an event assertion to the existing `check_status_works` test to
verify `BountyPayoutProcessed` emits the correct net payout value
(`parent_value - child_value`) instead of the full parent value. This
assertion fails with `take()` and passes with `get()`.

- **Current deployments (KAH, PAH)**: Both use `LocalPay` where
`check_payment()` always returns `Success`. The retry/lock path is
unreachable, but the `BountyPayoutProcessed` event emits an incorrect
payout value for parent bounties with child bounties.
- **Future deployments**: If the pallet is configured with an async
`Paymaster` (e.g., XCM-based) where `check_payment()` can return
`Failure`, the `retry_payment()` path would compute a wrong payout
amount, potentially leading to permanent fund lock with no recovery path
(since `close_bounty()` rejects `PayoutAttempted` status).

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 5fbeadd)
EgorPopelyaev pushed a commit that referenced this pull request Apr 10, 2026
Backport #11425 into `stable2603` from dhirajs0.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Dhiraj Sah <dhiraj@parity.io>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
EgorPopelyaev pushed a commit that referenced this pull request Apr 10, 2026
Backport #11425 into `stable2512` from dhirajs0.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Dhiraj Sah <dhiraj@parity.io>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A4-backport-stable2512 Pull request must be backported to the stable2512 release branch A4-backport-stable2603 Pull request must be backported to the stable2603 release branch T1-FRAME This PR/Issue is related to core FRAME, the framework.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants