-
Notifications
You must be signed in to change notification settings - Fork 305
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
Removes all usages of UnsignedLong usage and changes to long. #550
Conversation
ethereum/datastructures/src/main/java/tech/pegasys/artemis/datastructures/Constants.java
Outdated
Show resolved
Hide resolved
...atetransition/src/main/java/tech/pegasys/artemis/statetransition/util/SlotProcessorUtil.java
Show resolved
Hide resolved
ethereum/datastructures/src/main/java/tech/pegasys/artemis/datastructures/Constants.java
Outdated
Show resolved
Hide resolved
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.
A number of concerns about index out of bounds errors, loss of precision, and order of operations. I've still yet to review BlockProcessorUtil
, AttestationUtil
, BeaconStateUtil
, and BeaconState
, which I will do after the v0.5.1 upgrade, however I expect they will have similar issues.
ethereum/datastructures/src/main/java/tech/pegasys/artemis/datastructures/Constants.java
Outdated
Show resolved
Hide resolved
ethereum/datastructures/src/main/java/tech/pegasys/artemis/datastructures/Constants.java
Outdated
Show resolved
Hide resolved
ethereum/datastructures/src/main/java/tech/pegasys/artemis/datastructures/Constants.java
Outdated
Show resolved
Hide resolved
...tures/src/main/java/tech/pegasys/artemis/datastructures/operations/SlashableAttestation.java
Outdated
Show resolved
Hide resolved
...tures/src/main/java/tech/pegasys/artemis/datastructures/operations/SlashableAttestation.java
Outdated
Show resolved
Hide resolved
...tetransition/src/main/java/tech/pegasys/artemis/statetransition/util/EpochProcessorUtil.java
Outdated
Show resolved
Hide resolved
...tetransition/src/main/java/tech/pegasys/artemis/statetransition/util/EpochProcessorUtil.java
Outdated
Show resolved
Hide resolved
...tetransition/src/main/java/tech/pegasys/artemis/statetransition/util/EpochProcessorUtil.java
Outdated
Show resolved
Hide resolved
...tetransition/src/main/java/tech/pegasys/artemis/statetransition/util/EpochProcessorUtil.java
Outdated
Show resolved
Hide resolved
...tetransition/src/main/java/tech/pegasys/artemis/statetransition/util/EpochProcessorUtil.java
Show resolved
Hide resolved
Co-Authored-By: akhila-raju <[email protected]>
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.
Found some more order of operations (OOO) glitches.
...m/datastructures/src/main/java/tech/pegasys/artemis/datastructures/util/BeaconStateUtil.java
Outdated
Show resolved
Hide resolved
...m/datastructures/src/main/java/tech/pegasys/artemis/datastructures/util/BeaconStateUtil.java
Outdated
Show resolved
Hide resolved
...m/datastructures/src/main/java/tech/pegasys/artemis/datastructures/util/BeaconStateUtil.java
Outdated
Show resolved
Hide resolved
...tetransition/src/main/java/tech/pegasys/artemis/statetransition/util/EpochProcessorUtil.java
Outdated
Show resolved
Hide resolved
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.
LGTM, I don't see any remaining issues.
* Move from Cava to Apache Tuweni * Add snapshots repository even when building with jenkins
…es header to specify the datatype (Consensys#599)
* Fixed attestation typo AND am now using the return result from plumb tree of the message hash * changing message_type to attributes
PR Description
Removes all usages of UnsignedLong usage and changes to long.
Fixed Issue(s)
Fixes #312.