Skip to content
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

remove more int64 usage #2369

Merged
merged 2 commits into from
Mar 2, 2021
Merged

remove more int64 usage #2369

merged 2 commits into from
Mar 2, 2021

Conversation

tersec
Copy link
Contributor

@tersec tersec commented Mar 2, 2021

for i in max(1, newBlockSlot.int64 - ATTESTATION_LOOKBACK.int64) ..
newBlockSlot.int64:
for i in max(
1'u64, newBlockSlot - min(newBlockSlot, ATTESTATION_LOOKBACK)) ..
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we introduce some saturated math type?

As is, this needs a comment explaining why we complexify things, the bounds are quite tricky here:

  • inclusive end, which lead to off by 1
  • need to think about unsigned underflow
  • need to start at 1 and so need to filter out 0 from the range

Copy link
Contributor Author

Choose a reason for hiding this comment

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

8825b9a

There's already toGaugeValue() and clamp(); what would this saturated type add? Most of the remaining items in #2366 need non-saturating arithmetic, to the extent they're issues at all (e.g., I only included the genesis time one for completion).

@tersec tersec merged commit 2b5a3a6 into unstable Mar 2, 2021
@tersec tersec deleted the lee branch March 2, 2021 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants