Skip to content

[stable2512] Backport #11425#11622

Merged
EgorPopelyaev merged 5 commits intostable2512from
backport-11425-to-stable2512
Apr 10, 2026
Merged

[stable2512] Backport #11425#11622
EgorPopelyaev merged 5 commits intostable2512from
backport-11425-to-stable2512

Conversation

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

Backport #11425 into stable2512 from dhirajs0.

See the documentation on how to use this bot.

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

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

@github-actions github-actions Bot added the A3-backport Pull request is already reviewed well in another branch. label Apr 2, 2026
@github-actions github-actions Bot requested a review from dhirajs0 April 2, 2026 12:54
…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)
@dhirajs0 dhirajs0 force-pushed the backport-11425-to-stable2512 branch from 3ac06d4 to a9900c7 Compare April 7, 2026 17:32
@dhirajs0 dhirajs0 marked this pull request as ready for review April 7, 2026 17:32
@dhirajs0 dhirajs0 requested a review from a team as a code owner April 7, 2026 17:32
@dhirajs0 dhirajs0 requested review from Szegoo and muharem April 7, 2026 17:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 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.

@EgorPopelyaev EgorPopelyaev enabled auto-merge (squash) April 8, 2026 13:24
@EgorPopelyaev EgorPopelyaev merged commit 38a829c into stable2512 Apr 10, 2026
243 of 258 checks passed
@EgorPopelyaev EgorPopelyaev deleted the backport-11425-to-stable2512 branch April 10, 2026 13:50
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