Auto-renewal gets disabled after failed payment attempts

[ipapandinas]

Is there an existing issue?

• I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• This is not a support question.

Description of bug

When auto-renewal fails due to insufficient funds, the system disables the auto-renewal setting entirely and requires manual re-enablement. However, the documentation states:

"Even if an auto-renewal attempt fails, the auto-renewal setting remains active for subsequent sales. This means the setting persists across multiple periods once you've configured auto-renewal."

The actual behavior contradicts the documentation. This could create governance overhead to re-enable auto-renewal and risks missing renewal windows. We expected auto-renewal to remain enabled and retry on the next cycle as documented, not disable itself after payment failures.

The behavior likely stems from the broker pallet logic when checking Sovereign account funds, where failed renewal attempts return None disabling the auto-renewal setting.

polkadot-sdk/substrate/frame/broker/src/tick_impls.rs

Lines 354 to 360 in 13320f3

	let Some(payer) = T::SovereignAccountOf::maybe_convert(record.task) else {
	Self::deposit_event(Event::<T>::AutoRenewalFailed {
	core: record.core,
	payer: None,
	});
	return None
	};

Is this disable-on-failure behavior intentional, or should it match the documented behavior where auto-renewal persists through payment failures?

Steps to reproduce

1. Enable auto-renewal on a core
2. Ensure Sovereign account has insufficient funds for renewal
3. Wait for the renewal attempt at the beginning of the new sales period
4. Observe auto-renewal setting becomes disabled

[seadanda]

Disable-on-failure was the intention of the original PR, the docs are incorrect.

I think we could (/should) handle this more elegantly in the code though, with the entry staying in autorenewals for a certain number of failed auto-renews sales before being ejected, or even something like a couple of retries a day apart so that the account can be funded during the sale.

[agustinustheo]

Hey @seadanda can I attempt this?

Edit:

I think after reading the code, for this specific code snippet, the logical thing to do is to exit since the payer is None.

polkadot-sdk/substrate/frame/broker/src/tick_impls.rs

Lines 354 to 360 in 13320f3

	let Some(payer) = T::SovereignAccountOf::maybe_convert(record.task) else {
	Self::deposit_event(Event::<T>::AutoRenewalFailed {
	core: record.core,
	payer: None,
	});
	return None
	};

But for the following code snippet:

polkadot-sdk/substrate/frame/broker/src/tick_impls.rs

Lines 369 to 374 in 13320f3

	Self::deposit_event(Event::<T>::AutoRenewalFailed {
	core: record.core,
	payer: Some(payer),
	});
	None

We can try to implement an elegant way for the entry to stay in autorenewals. There would need to be a new storage map in order to look up if the corresponding payer address failed an auto-renewal or not.

polkadot-sdk/substrate/frame/broker/src/lib.rs

Lines 197 to 199 in 13320f3

	#[pallet::storage]
	pub type AutoRenewals<T: Config> =
	StorageValue<_, BoundedVec<AutoRenewalRecord, T::MaxAutoRenewals>, ValueQuery>;

My next question would be, if we do want to implement retries, how many retries should we give the payer before we get assign the auto-renewal as Failed?

[bkchr]

Hey @seadanda can I attempt this?

Yes feel free to take it.

My next question would be, if we do want to implement retries, how many retries should we give the payer before we get assign the auto-renewal as Failed?

Just make it a configuration parameter.

The code you found there is the correct code and you would need to modify this. The best is you start with a test that demonstrates the wanted behavior.