ff cleanup: reduce_stake_warmup_cooldown

[AshwinSekar]

This feature has been activated on all clusters.

There are a couple more uses that we cannot yet remove, in order for historical stake information to function properly:

agave/cli/src/stake.rs

Lines 2573 to 2576 in 182d27f

	let new_rate_activation_epoch = get_feature_activation_epoch(
	rpc_client,
	&feature_set::reduce_stake_warmup_cooldown::id(),
	)?;

agave/cli/src/cluster_query.rs

Lines 1830 to 1831 in 182d27f

	let new_rate_activation_epoch =
	get_feature_activation_epoch(rpc_client, &feature_set::reduce_stake_warmup_cooldown::id())?;

agave/sdk/src/feature_set.rs

Lines 1079 to 1082 in 182d27f

	pub fn new_warmup_cooldown_rate_epoch(&self, epoch_schedule: &EpochSchedule) -> Option<Epoch> {
	self.activated_slot(&reduce_stake_warmup_cooldown::id())
	.map(|slot| epoch_schedule.get_epoch(slot))
	}

Fixes solana-labs#32724

[joncinque]

Just a comment on removing the account from the interface too.

cc @2501babe for awareness, who's been working on the BPF port of the stake program

[joncinque]

The single pool program has a test that swaps out every account in an instruction to make sure that it fails. With this change, the config account doesn't matter, so swapping the stake config with a random account now succeeds.

Fixed with solana-labs/solana-program-library#6543

Further work to be done with solana-labs/solana-program-library#6542 once this change is available in a crate. We can't do it right away because the instruction creators on 1.18 still reference the stake config account.

[joncinque]

Bah darn, looks like there's one more test that breaks. Let me fix that up...

[joncinque]

Sorry, we have to revert my suggestion. The downstream breakage is legit -- in the old version, the instruction creator gives the stake config account, and in the new version, it gives the system program. So when a downstream program uses those instruction creators from the stake program, then its own instruction creators also have to take in the stake config account.

When they upgrade with this change, their program is still receiving the stake config account, but the stake instruction creator expects the system program in that place, causing an error during CPI.

We could say "2.0 is breaking, tough luck", but instant breakage is a bad experience, and there's no clean way for SPL to support both at once.

Again, I'm really sorry that I asked you to make that painful change. I should have thought a bit further about the consequences.

[AshwinSekar]

No worries! so for a fix, should we revert the instruction creator to use the stake config account?
Or add a new creator which uses the system account, and deprecate the old creator in hopes of removing it once downstream projects stop using it?

[joncinque]

Let's just revert the change. We can deprecate them when we create totally new instruction variants

[joncinque]

And sorry, to be totally clear, I mean reverting my suggestions, the first commit is good

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (0168e0a) to head (84e2dfa).
Report is 11 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master     #470     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         847      849      +2     
  Lines      229180   229072    -108     
=========================================
- Hits       187600   187463    -137     
- Misses      41580    41609     +29