-
Notifications
You must be signed in to change notification settings - Fork 225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(fortuna): Add a couple more config values #2251
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
/// The fee will be lowered if it is more profitable than specified here. | ||
/// Must be larger than min_profit_pct. | ||
pub max_profit_pct: u64, | ||
/// The minimum value for this is -100. If set to < 0, it means the keeper may lose money on callbacks that use the full gas limit. | ||
pub max_profit_pct: i64, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(In hindsight, I wish that i had specified these values as multipliers, so 0 would be the natural lower bound and you could just say "75" instead of "-25". That's how I did the other scalars and it would have been better here.
However, I'm choosing to support up to -100 now in order to maintain backward compatibility on the config format)
apps/fortuna/src/keeper.rs
Outdated
chain_eth_config.target_profit_pct, | ||
chain_eth_config.max_profit_pct, | ||
// NOTE: unwrap() here so we panic early if someone configures these values below -100. | ||
u64::try_from(100 + chain_eth_config.min_profit_pct).unwrap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just switch to expect to give more context
Two simple changes here