pallet-bounties: allow bounties to never expire by BigTava · Pull Request #7723 · paritytech/polkadot-sdk

BigTava · 2025-02-26T10:41:52Z

Description

The Bounties expiration and renewal heavily depends on manual interactions through UI. This PR refactors the update_due calculation to use saturating_add, allowing bounties to remain active indefinitely without requiring extend_bounty_expiry and preventing automatic curator slashing for inactivity, which often penalises unnecessarily. Previously, setting BountyUpdatePeriod to a large value, such as BlockNumber::max_value(), could cause an overflow.

Integration

Set BountyUpdatePeriod to BlockNumber::max_value()

Review Notes

Modifies how bounty expiry is handled

🔍 Code Diff Summary

- *update_due = Self::treasury_block_number() + T::BountyUpdatePeriod::get();
+ *update_due = Self::treasury_block_number().saturating_add(T::BountyUpdatePeriod::get());

… is None

muharem · 2025-02-27T14:21:50Z

prdoc/pr_7723.prdoc

+title: '[pallet-bounties] Make BountyUpdatePeriod optional'
+doc:
+- audience: Runtime Dev
+  description: Refactored `BountyUpdatePeriod` to be optional and adjusted expiration handling to default to `BlockNumber::MAX` when `None`.


this will be included in the release notes. I would explain what this means. this is more like a description of the implementation, it can be read from the code.

also please add PR description

Updated. Thanks!

muharem · 2025-02-27T14:24:05Z

substrate/frame/bounties/src/lib.rs

 		type BountyDepositPayoutDelay: Get<BlockNumberFor<Self, I>>;

-		/// Bounty duration in blocks.
+		/// Optional bounty duration in blocks. If `None`, it is considered `BlockNumber::MAX`.


it was a poor doc for this parameter, lets give more context. explain behaviour.
".. it is considered BlockNumber::MAX" is description of the implementation. what it means for a user.

Updated. Thanks!

muharem · 2025-02-27T16:21:23Z

prdoc/pr_7723.prdoc

+    If slashing of curators is necessary, it can be handled through OpenGov.
+    This reduces friction in curator management. 


Suggested change

If slashing of curators is necessary, it can be handled through OpenGov.

This reduces friction in curator management.

this is a bit specific to Polkadot, but the rest looks good

muharem · 2025-02-27T16:28:39Z

substrate/frame/bounties/src/lib.rs

 		type BountyDepositPayoutDelay: Get<BlockNumberFor<Self, I>>;

-		/// Bounty duration in blocks.
+		/// Optional bounty duration in blocks.


Suggested change

/// Optional bounty duration in blocks.

/// The period that starts when a curator is approved, during which they must execute or

/// update the bounty via `extend_bounty_expiry`. If missed, the bounty expires, and the

/// curator may be slashed.

Thanks for proposing. I think it is too long for the first sentence, given the guide "Strive to make the first sentence succint and short ...". But I will commit

muharem · 2025-02-27T16:29:33Z

substrate/frame/bounties/src/lib.rs

+		/// If `None`, bounties stay active indefinitely, removing the need for
+		/// `extend_bounty_expiry` and restricting curator slashing to OpenGov.


Suggested change

/// If `None`, bounties stay active indefinitely, removing the need for

/// `extend_bounty_expiry` and restricting curator slashing to OpenGov.

/// If `None`, bounties stay active indefinitely, removing the need for

/// `extend_bounty_expiry`.

OpenGov part is specific to the runtime e

…None

muharem · 2025-02-28T11:23:18Z

prdoc/pr_7723.prdoc

+        Migrating `BountyUpdatePeriod` from `BlockNumber` to `Option<BlockNumber>` where
+        if set to (), bounties will no longer expire.


set to None, () is a unit type. migration is more meant to guide clients who already using this feature and will face the breaking change. here the migration for them is simple, they need to wrap their value into Some(BountyUpdatePeriod) and that would be lead to exact same behaviour

Awesome feedback!

muharem · 2025-02-28T11:23:45Z

substrate/frame/bounties/src/lib.rs

+							T::BountyUpdatePeriod::get()
+								.unwrap_or(BlockNumberFor::<T, I>::max_value()),


this part repeated few time, you could abstract it into function

Great suggestion.

…alculation

github-actions · 2025-02-28T11:47:57Z

Review required! Latest push from author must always be reviewed

BigTava · 2025-02-28T11:54:39Z

@muharem I pushed again to apply your feedback. Who do you recommend for a second reviewer?

muharem · 2025-03-17T14:02:57Z

substrate/frame/child-bounties/src/benchmarking.rs

+
+		// If `BountyUpdatePeriod` overflows the inactivity timeout the benchmark still executes the slash
+		let origin = if Pallet::<T>::treasury_block_number() <= inactivity_timeout {
+			T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?


it should not be Weightless, even if you have not reject origin, the call is still executable by some caller in some cases or by curator

Great catch!

it should be curator when now <= inactivity_timeout, only curator in this case can run unassign_curator call if the bounty is active

…available

AKJUS

doing the same well...

paritytech-workflow-stopper · 2025-03-17T17:49:25Z

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/13905994866
Failed job name: test-linux-stable-runtime-benchmarks

AKJUS

given jasper said pull_request than...

* master: (58 commits) Upgrade link-checker cache to v4 (#7874) Updating readmes (#7950) Cumulus: Remove some old scripts (#7946) pallet-bounties: allow bounties to never expire (#7723) run frame-omni-bencher overhead command in CI for all runtimes in the runtime matrix (#7459) Update README.md for Cumulus (#7930) FRAME: Meta Transaction (#6428) Follow up for: Use the umbrella crate for the parachain template #5993 (#7464) Add an extra_constant to pallet-treasury (#7918) Bump the ci_dependencies group across 1 directory with 4 updates (#7855) remove compromised action (#7934) Fixing token-economics dead link (#5302) [pallet-revive] Fix pallet-revive-fixtures build.rs (#7928) cumulus: fix pov exporter format (#7923) sp-api: Support `mut` in `impl_runtime_apis!` (#7924) Remove clones from block seal function (#7917) [pallet-revive] precompiles 2->9 (#7810) Use non-native token to benchmark xcm on asset hub (#7893) [CI] bump timeout wait for build in zombienet workflows. (#7871) taplo: split long array line to multiline array (#7905) ...

Extend bounty update period to 10 years There have been multiple cases (e.g., [referenda/1588](https://polkadot.subsquare.io/referenda/1588)) where active bounties expired, requiring the community to vote again to reintroduce the same bounty. This PR increases the update period to 10 years to prevent such issues for newly created bounties. Note that the community can still cancel any active bounty through a governance vote. Once [polkadot-sdk#7723](paritytech/polkadot-sdk#7723) is included in an SDK release and adopted in runtime upgrades, we will be able to configure bounties to never expire. --------- Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: Adrian Catangiu <[email protected]>

pallet-bounties: extends update_due infinitly if BountiesUpdatePeriod…

52698ec

… is None

BigTava requested a review from a team as a code owner February 26, 2025 10:41

pallet-bounties: fmt

fa19b53

BigTava added the T2-pallets This PR/Issue is related to a particular pallet. label Feb 26, 2025

BigTava added 5 commits February 26, 2025 17:41

pallet-bounties: changes docs for config parameter and adds prdoc

4e49830

pallet-bounties: fix benchmarking

08c637d

Merge branch 'master' into tiago-remove-bounties-expiry

2478bfc

pallet-child-bounties: fix benchamarking

c8cb79e

pallet-child-bounties: move Bounded from lib to benchmarking

4654163

muharem reviewed Feb 27, 2025

View reviewed changes

pallet-bounties: improves prdoc and parameter docs

2e22a58

paritytech-review-bot bot requested a review from a team February 27, 2025 15:48

muharem reviewed Feb 27, 2025

View reviewed changes

BigTava added 2 commits February 27, 2025 17:43

pallet-bounties: refactors prdoc and tests BountyUpdatePeriod set to …

3890ab0

…None

docs: provides migration path

91eb76d

paritytech-review-bot bot requested a review from a team February 27, 2025 17:59

pallet-bounties: remove debug statements

2ef5617

muharem reviewed Feb 28, 2025

View reviewed changes

muharem approved these changes Feb 28, 2025

View reviewed changes

docs: improves migration path and adds function to reuse update_due c…

1ee5b56

…alculation

github-actions bot requested a review from muharem February 28, 2025 11:47

pallet-bounties: fmt

a927617

BigTava added 2 commits February 28, 2025 12:01

pallet-bounties: adds docs to calculate_update_due

cda4673

docs: adds API change in migration

5c18edd

muharem approved these changes Mar 2, 2025

View reviewed changes

github-actions bot requested a review from muharem March 17, 2025 12:35

muharem reviewed Mar 17, 2025

View reviewed changes

pallet-bounties: defaults to some signed caller if RejectOrigin is un…

3bde188

…available

github-actions bot requested a review from muharem March 17, 2025 15:03

AKJUS approved these changes Mar 17, 2025

View reviewed changes

pallet-bounties: defaults to curator if RejectOrigin is unavailable

e754423

pallet-child-bounties: minor

0b8c2a5

muharem approved these changes Mar 17, 2025

View reviewed changes

BigTava requested a review from AKJUS March 17, 2025 18:26

bkchr approved these changes Mar 17, 2025

View reviewed changes

AKJUS approved these changes Mar 18, 2025

View reviewed changes

BigTava added this pull request to the merge queue Mar 18, 2025

Merged via the queue into master with commit e4f7814 Mar 18, 2025
253 of 260 checks passed

BigTava deleted the tiago-remove-bounties-expiry branch March 18, 2025 10:56

ggwpez mentioned this pull request Jun 4, 2025

Bump to stable2503 polkadot-fellows/runtimes#711

Merged

7 tasks

github-actions bot mentioned this pull request Jun 10, 2025

Update polkadot-sdk from stable2412 to stable2503 moondance-labs/tanssi#1087

Closed

muharem mentioned this pull request Jun 10, 2025

Extend bounty update period to 10 years polkadot-fellows/runtimes#766

Merged

This was referenced Jul 16, 2025

Update polkadot-sdk from stable2412 to stable2503 moonbeam-foundation/moonbeam#3362

Closed

Update polkadot-sdk from stable2412 to stable2503-7 moonbeam-foundation/moonbeam#3363

Closed

pgherveou added this to [preview] release tracker Mar 20, 2026

github-project-automation bot moved this to Todo in [preview] release tracker Mar 20, 2026

github-project-automation bot moved this from Todo to Done in [preview] release tracker Mar 20, 2026

pgherveou removed this from [preview] release tracker Mar 20, 2026

pgherveou added this to [preview] release tracker Mar 23, 2026

github-project-automation bot moved this to Todo in [preview] release tracker Mar 23, 2026

github-project-automation bot moved this from Todo to Done in [preview] release tracker Mar 23, 2026

		If slashing of curators is necessary, it can be handled through OpenGov.
		This reduces friction in curator management.

-		/// Optional bounty duration in blocks.
+		/// The period that starts when a curator is approved, during which they must execute or
+		/// update the bounty via `extend_bounty_expiry`. If missed, the bounty expires, and the
+		/// curator may be slashed.

		/// If `None`, bounties stay active indefinitely, removing the need for
		/// `extend_bounty_expiry` and restricting curator slashing to OpenGov.

		Migrating `BountyUpdatePeriod` from `BlockNumber` to `Option<BlockNumber>` where
		if set to (), bounties will no longer expire.

		T::BountyUpdatePeriod::get()
		.unwrap_or(BlockNumberFor::<T, I>::max_value()),

Conversation

BigTava commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Integration

Review Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 28, 2025

Uh oh!

BigTava commented Feb 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AKJUS left a comment

Choose a reason for hiding this comment

Uh oh!

paritytech-workflow-stopper bot commented Mar 17, 2025

Uh oh!

AKJUS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

BigTava commented Feb 26, 2025 •

edited

Loading