Multiple fixes around commitments and group keys #594
Merged
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
In this commit, we attempt a simple sorting routine to address: #591. Without these, we'll try to validate transfer proofs out of order, which is akin verifying an orphan transaction in the Bitcoin mempool. Rather than do a full on topological sort, we instead just sort by block height. As we don't support spending unconfirmed change, this should be enough to serialize the contents of a block.
For easier debugging, it's useful to have a nice list of all asset IDs of assets minted during an integration test.
Fixes #582. We forgot to respect the group key of an asset when burning, which lead to the "input commitment does not contain asset_id=..." error.
guggero
force-pushed
the
load-test-rpc-proof-courier
branch
from
866f574
to
ecf3241
Compare
Roasbeef
reviewed
Oct 17, 2023
This fixes an error where index 0 is not equal to the recipient's index. This is only the case for burns so it wasn't detected before. For a burn, we always have two outputs: One for the split root and one for the actual burn. If a split root isn't needed because it's a burn of the full amount (and we're just carrying passive assets), we actually remove the first output again. But that happens _after_ this code, so we need to make sure we use the correct output.
This commit fixes an issue with the new type TypeSimplePassiveAssets that we added recently for burns. That type also needs to carry passive assets in its output.
With this commit we properly set the asset version for burns and when creating the actual asset outputs.
Once an asset from a split becomes a passive asset in the next transfer, we can actually drop the split commitment. We did that in the commitment but not in the database, which resulted in the split commitment being present again in the input commitment (which lead to a root key mismatch). This wasn't discovered before because we didn't have a test that carried an asset resulting from a split as a passive asset in the next transfer.
This commit adds two more burns (burning a grouped normal asset and burning a grouped collectible) to the burn itest. This updated test found all the bugs fixed in the previous commits.
Fixes sending grouped asset proofs over the RPC proof courier by correctly setting the group key when necessary.
This is the change in the load test that discovered the bug fixed in the previous commit where grouped asset proofs couldn't be transferred properly over the RPC proof courier.
This itest makes sure we can send proofs for grouped assets properly over the RPC proof courier.
Since we're closing the fetchedLeaves channel right below this call, it's very likely that we run into a "send on closed channel" panic. We fix that by making the send synchronous.
guggero
force-pushed
the
load-test-rpc-proof-courier
branch
from
ecf3241
to
33eeafb
Compare
jharveyb
approved these changes
Oct 17, 2023
We add more context to each error site, making it easier to debug.
We add more detail to each error site, making it easier to debug.
No reason to not just return the error directly here. Callers should be using errors.Is and errors.As.
In this commit we fix two bugs:
1. Before RegisterNewIssuanceBatch didn't properly attempt to look up
the prev input for items in a batch. This caused the transfer leaf
validation to fail, bailing the whole thing.
2. When validating items of a batch, inputs spent in the batch may
have been produced within the batch itself. As a result, the look
up for the local universe tree will fail, as we're still validating
the batch. This is similar to needing to check for inputs spent in
a block before assuming the input doesn't exist. To fix this, we
make a new set of batch deps, then check that before we consult our
local universe.
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.
This is a collection of bug fixes and the corresponding integration tests that discovered them.
Depends on #592.
Fixes #582.
Fixes #571.
Fixes #593.