Fix tests w/ sending outputs as subset of receiving #7
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
Co-authored-by: RubenSomsen <[email protected]>
Co-authored-by: Vojtěch Strnad <[email protected]>
BIP173 defines a 90 char limit, but we need 115. Most implementations already remove this limit as lightning encodings are longer than 90 char and use bech32, but still should be mentioned for wallet implementors.
This better matches conventions used in other BIPS. It's also likely wallet software already has code for doing these operations if they support BIP32.
Remove version since upgrades to silent payments not require regenerating silent payment keys. If there is a future upgrade that would require regenerating keys, then it should have a new BIP number.
Instead of 32-byte x-only keys, use compressed public keys for the silent payments address. This is to avoid parity mismatches, especially in the case of adding labels to the spend public key, which will cause the sign of the y point to flip between even and odd. Without the extra byte, we would need the sender and receiver to constantly negate the public and private keys at each step, which adds complexity and computational expense.
If Alice's wallet supports receiving silent payments, they can use the silent payment protocol to create change outputs for themselves. Mention this explicitly in the specification for sending.
Include a modified copy of bech32m (removes the 90 char limit) and a copy of the python secp256k1 implementation from the Bitcoin Core test framework
In order to maintain parity, negate the private keys for taproot outputs before performing ECDH. This ensures the receiver will arrive at the same result without needing to check both the even and odd y points for taproot x-only public keys
Detail backwards compatible upgrades vs backwards incompatible upgrades
josibake
force-pushed
the
silent-payments-bip
branch
11 times, most recently
from
91bf059 to
478a5c5
Compare
josibake
force-pushed
the
silent-payments-bip
branch
2 times, most recently
from
645c871 to
c55f80c
Compare
josibake
force-pushed
the
silent-payments-bip
branch
3 times, most recently
from
eece921 to
f0625f7
Compare
josibake
force-pushed
the
silent-payments-bip
branch
4 times, most recently
from
b349517 to
d73e924
Compare
josibake
force-pushed
the
silent-payments-bip
branch
2 times, most recently
from
30cdd03 to
a2b52fe
Compare
josibake
force-pushed
the
silent-payments-bip
branch
from
1e10265 to
9912b88
Compare
josibake
force-pushed
the
silent-payments-bip
branch
from
73f1a52 to
359fa75
Compare
josibake
force-pushed
the
silent-payments-bip
branch
from
2857784 to
92b12ab
Compare
josibake
force-pushed
the
silent-payments-bip
branch
from
92b12ab to
f6dd067
Compare
josibake
force-pushed
the
silent-payments-bip
branch
2 times, most recently
from
9c82669 to
57c89ae
Compare
josibake
force-pushed
the
silent-payments-bip
branch
3 times, most recently
from
d8ed18c to
36fefe5
Compare
josibake
force-pushed
the
silent-payments-bip
branch
3 times, most recently
from
a60bfc3 to
17e1d16
Compare
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.
I was having trouble running the tests, it failed at "Multiple outputs: multiple outputs, same recipient". After looking at the test logic and cygnet3 implementation, I noticed this slight difference between the python code and his rust. It seems to make sense to me and after that all tests pass.