crypto: rework action-level EffectingData implementations
#2931
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For spends, only the body is considered effecting data, so we just hash the proto-encoded body.
For DAO spends, outputs, and deposits, the entire action is considered effecting data.
redshiftzero
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
Note that previously the batch swap output data was _not_ included in the effect hash for the swap claim.
redshiftzero
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
For delegate and undelegate actions, the effecting data should be the entire action. For undelgate claims however, the effecting data should just be the body, thus excluding the proof which is on the undelegate claim action.
redshiftzero
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
The `PositionClose`, `PositionWithdraw`, and `PositionRewardClaim` actions effect hash still hashes the same data - but now first proto-encoded. The `PositionOpen` effect hash is slightly modified. Previously the `PositionOpen` effect hash consisted of: * The reserves R_1 and R_2 * The ID of the position, which is a hash computed from the nonce, asset IDs of the trading pair, the fee, and p and q Now, by using the proto-encoded position, the effect hash includes the: * State, which is an enum determining if the position is opened, closed, claimed, or withdrawn (new) * Reserves R_1 and R_2 * Phi (trading function), which means the fee, p, and q, as well as the trading pair (two asset IDs) * Nonce * A `close_on_fill` boolean indicating if this position is a limit order (new) The `PositionOpen` effect hash no longer includes the ID.
redshiftzero
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
redshiftzero
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
redshiftzero
changed the title
EffectingData implementationsEffectingData implementations
cronokirby
reviewed
Aug 17, 2023
cronokirby
reviewed
Aug 17, 2023
cronokirby
reviewed
Aug 17, 2023
There was a problem hiding this comment.
Looks straightforward to me; I think we can probably add a helper function for the same 3 repeated lines of logic, would make things a bit cleaner.
Also, one thing I haven't really reviewed is whether or not we haven't missed any relevant structs with this set of changes.
redshiftzero
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
redshiftzero
changed the title
EffectingData implementationsEffectingData implementations
redshiftzero
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
redshiftzero
force-pushed
the
2000-proto-effect-hash
branch
from
b2addcc
to
cdbfac7
Compare
redshiftzero
temporarily deployed
to
smoke-test
GitHub Actions
Inactive
redshiftzero
changed the title
EffectingData implementationsEffectingData implementations
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Towards one of the checkboxes in #2808
Closes #2000
The scope of this PR is to in each
EffectingDataimplementation hash a single proto that represents the effecting data of that transaction action.