Skip to content

Conversation

@grod220
Copy link
Member

@grod220 grod220 commented Oct 31, 2025

This pull request implements SIMD-0391, replacing all floating-point (f64) arithmetic in the Stake Program's warmup and cooldown logic with a fixed-point implementation using integer arithmetic. See SIMD for background rationale.

What's new?

  • The warmup/cooldown rate calculation now uses BPS values and u128 integer arithmetic.
  • All logic related to this now in isolated warmup_cooldown_allowance module
  • Delegation struct has been updated to maintain backward compatibility with on-chain data. The warmup_cooldown_rate: f64 field is replaced with _reserved: [u8; 8] (preserves memory layout)

Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall! Mostly nits and a few suggestions

@grod220 grod220 force-pushed the float-to-fixed-point branch from 009dc87 to 9c0ba5c Compare November 18, 2025 12:47
@grod220 grod220 requested a review from joncinque November 18, 2025 12:59
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good! The proptests are excellent, just some bits about the deprecation strategy

@grod220
Copy link
Member Author

grod220 commented Nov 25, 2025

Spending a bit of time thinking of this:

// Overflowing u128 is not a realistic scenario except in tests. However, in that case
// it's reasonable to allow activation/deactivation of the account's entire portion.
None => account_portion,

Going to run a few mainnet scenarios. I have a hunch we should saturate the numerators instead.

joncinque
joncinque previously approved these changes Nov 25, 2025
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ready to me! Feel free to do any other testing you want

@grod220
Copy link
Member Author

grod220 commented Nov 25, 2025

@joncinque can you have a look at the latest update? I think saturating the numerators is safer in the extreme cases versus defaulting to the entire account_portion. These cases are not super realistic, but feels more reassuring that there isn't a rate or sizing that could subvert the rate-limit protections. The SIMD would also need updating to reflect this behavior.

@grod220 grod220 requested a review from joncinque November 25, 2025 12:24
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me! Just a nit and a question

@grod220 grod220 requested a review from joncinque December 1, 2025 09:35
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me, thanks for addressing all the comments!

Copy link

@rustopian rustopian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent testing approach 🦾

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants