Cleanup X-chain UniqueTx Dependencies #1656
Merged
Conversation
This file contains hidden or 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
StephenButtolph
added
vm
StephenButtolph
commented
Jun 26, 2023
3 tasks
StephenButtolph
commented
Jun 27, 2023
Comment on lines
-237
to
-243
| assets := tx.AssetIDs() | ||
| require.Len(assets, 1) | ||
| require.Contains(assets, assetID) | ||
|
|
||
| consumedAssets := tx.ConsumedAssetIDs() | ||
| require.Len(consumedAssets, 1) | ||
| require.Contains(consumedAssets, assetID) |
There was a problem hiding this comment.
These functions no longer exist
Comment on lines
+398
to
+400
| tx0 := &testTx{ | ||
| Tx: innerTx0, | ||
| tx: tx1, |
There was a problem hiding this comment.
This updates the dependencies of tx1 to be empty after tx0 is accepted.
| @@ -54,26 +54,6 @@ func (t *OperationTx) InputIDs() set.Set[ids.ID] { | |||
| return inputs | |||
| } | |||
|
|
|||
| // ConsumedAssetIDs returns the IDs of the assets this transaction consumes | |||
| func (t *OperationTx) ConsumedAssetIDs() set.Set[ids.ID] { | |||
| assets := t.AssetIDs() | |||
There was a problem hiding this comment.
This was incorrect before. It should have been assets := t.BaseTx.AssetIDs().
| }) | ||
| if inputTx.Status() != choices.Accepted { | ||
| txIDs.Add(txID) | ||
| } | ||
| } |
There was a problem hiding this comment.
This loop was only needed if someone created a Fx with an Operation that could be issued with no Input UTXOs. However, no such Fx was ever added - and so it is safe to remove (as all chains are now linearized).
StephenButtolph
commented
Jun 27, 2023
Comment on lines
-169
to
-171
| if tx.Tx == nil || len(tx.deps) != 0 { | ||
| return tx.deps, nil | ||
| } |
There was a problem hiding this comment.
Note: tx.Tx can never be nil here because MissingDependencies() is only called from txs returned from ParseTx (which can clearly never have a nil tx.Tx).
StephenButtolph
requested review from
coffeeavax,
gyuho and
danlaine
as code owners
coffeeavax
approved these changes
Jun 27, 2023
hexfusion
approved these changes
Jun 27, 2023
gyuho
approved these changes
Jun 27, 2023
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.
Why this should be merged
Continuing the push to make
UniqueTxas minimal as possible. Once we removeStatuses from being persisted my conquest will abide here.How this works
How this was tested