Skip to content

[stable2603] Backport #11425#11623

Merged
EgorPopelyaev merged 4 commits intostable2603from
backport-11425-to-stable2603
Apr 10, 2026
Merged

[stable2603] Backport #11425#11623
EgorPopelyaev merged 4 commits intostable2603from
backport-11425-to-stable2603

Conversation

@paritytech-release-backport-bot
Copy link
Copy Markdown

Backport #11425 into stable2603 from dhirajs0.

See the documentation on how to use this bot.

…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)
@github-actions github-actions Bot added the A3-backport Pull request is already reviewed well in another branch. label Apr 2, 2026
@paritytech-release-backport-bot paritytech-release-backport-bot Bot requested a review from a team as a code owner April 2, 2026 12:54
@github-actions github-actions Bot requested a review from dhirajs0 April 2, 2026 12:54
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

This pull request is amending an existing release. Please proceed with extreme caution,
as to not impact downstream teams that rely on the stability of it. Some things to consider:

  • Backports are only for 'patch' or 'minor' changes. No 'major' or other breaking change.
  • Should be a legit fix for some bug, not adding tons of new features.
  • Must either be already audited or not need an audit.
Emergency Bypass

If you really need to bypass this check: add validate: false to each crate
in the Prdoc where a breaking change is introduced. This will release a new major
version of that crate and all its reverse dependencies and basically break the release.

@dhirajs0 dhirajs0 requested review from Szegoo and muharem April 7, 2026 17:35
@EgorPopelyaev EgorPopelyaev enabled auto-merge (squash) April 8, 2026 13:36
@EgorPopelyaev EgorPopelyaev merged commit 58d16f8 into stable2603 Apr 10, 2026
248 of 251 checks passed
@EgorPopelyaev EgorPopelyaev deleted the backport-11425-to-stable2603 branch April 10, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A3-backport Pull request is already reviewed well in another branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants