-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add skip participants with too low balance. Closes #42 #46
Conversation
bin/release-rewards.js
Outdated
if (amount === 0) return | ||
const totalScheduledRewards = | ||
(await ie.rewardsScheduledFor(address)) + amount | ||
if (totalScheduledRewards >= 0.1e18) { |
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.
I think the following would be easier to read:
if (totalScheduledRewards >= 0.1e18) { | |
if (totalScheduledRewards >= 0.1 * 1e18) { |
Do you have any concerns about comparing a BigInt value against a float value and potentially losing precision?
A possible solution that avoids the problem:
if (totalScheduledRewards >= 0.1e18) { | |
if (totalScheduledRewards >= /* 0.1 FIL */ 10n ** 18n / 10n) { |
But maybe this is not an issue.
❯ node
Welcome to Node.js v20.11.1.
Type ".help" for more information.
> 10n ** 18n / 10n
100000000000000000n
> 0.1e18
100000000000000000
>
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.
I don't have an opinion on this, as the results are the same and for me the current version is easiest to read. What do you prefer?
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.
I find 0.1 * 1e18
easier to read than 0.1e18
- I have to think a bit to decipher what 0.1e18
means.
Adding a code comment explaining the threshold is 0.1 FIL
is probably the best solution, as it does not require any knowledge about attoFIL being
Anyhow, this is a detail not worth bikeshedding. Choose the option you prefer yourself!
c6dbd44
to
7e6dfb4
Compare
bin/release-rewards.js
Outdated
if (amount === 0) return | ||
const totalScheduledRewards = | ||
(await ie.rewardsScheduledFor(address)) + amount | ||
if (totalScheduledRewards >= 0.1e18) { |
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.
I find 0.1 * 1e18
easier to read than 0.1e18
- I have to think a bit to decipher what 0.1e18
means.
Adding a code comment explaining the threshold is 0.1 FIL
is probably the best solution, as it does not require any knowledge about attoFIL being
Anyhow, this is a detail not worth bikeshedding. Choose the option you prefer yourself!
Cool, I've changed it to |
Closes #42