[group key addrs 6/7]: bug fixes and refactors #1658
+877
−472
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
To allow a sender to re-try sending a message easily, we return the same response for a message that uses the same proof and receiver key as if we saw that message for the first time. We can't actually check the content of the message, since it's encrypted, and might differ each time it is re-encrypted, even if the cleartext remains the same. Since with the address v2 scheme each outpoint can only have a single recipient, we can make the assumption that a message that claims the same outpoint and is for the same recipient also is the same message.
We'll want to use the same error being returned in all asset group related database interactions.
For V2 addresses we need to know certain information from the address when creating the final send fragment. Because we might be transporting a vPSBT over RPC boundaries between funding and finalizing, we need to also transport the address between those steps. The easiest way to do that is to encode the address into the virtual output directly.
We'll need to know the last block height we processed for any event based on the address version.
This was always a bit confusing and also unnecessarily complicated. We'll need to manually re-create the tapscript root given a sibling in the next commit, where we'll use similar code. This refactor shows that the root with no sibling can easily be transformed into the root with a sibling.
guggero
force-pushed
the
more-group-key-refactors
branch
from
fbf18b2 to
d40b977
Compare
Pull Request Test Coverage Report for Build 16373392934Details
💛 - Coveralls |
ffranr
approved these changes
Jul 21, 2025
| // It's a different recipient, so someone is | ||
| // attempting to re-use a proof for a different | ||
| // recipient. | ||
| return nil, proof.ErrTxMerkleProofExists |
There was a problem hiding this comment.
returned error type here doesn't make sense to me. I would expect an error type along the lines of: attempting to reuse outpoint for a different receiver.
| @@ -174,3 +174,12 @@ WHERE addr_events.status >= @status_from | |||
| AND COALESCE(@addr_taproot_key, addrs.taproot_output_key) = addrs.taproot_output_key | |||
| AND addr_events.creation_time >= @created_after | |||
| ORDER by addr_events.creation_time; | |||
|
|
|||
| -- name: QueryLastEventHeightByAddrVersion :one | |||
There was a problem hiding this comment.
I think we should just call this QueryLastEventHeight and the addr version could be thought of as an optional argument. But not a big deal.
Comment on lines
+170
to
+173
| // The exclusive start message ID, meaning messages with this ID or higher | ||
| // will be included in the response. This allows the client to resume | ||
| // receiving messages from a specific point without missing any. One of | ||
| // start_message_id_exclusive, start_block_height_inclusive or |
There was a problem hiding this comment.
Would be good to add doc comment for bytes receiver_id = 1; above also
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.
Extracted some commits from #1587 as suggested.
Addressed comments on existing review in #1587 that were meant for these commits.