Static invoice server prefactor #3667
Open
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
|
👋 Thanks for assigning @jkczyz as a reviewer! |
valentinewallace
force-pushed
the
2025-03-inv-server-prefactor
branch
2 times, most recently
from
165c88d to
9079c41
Compare
valentinewallace
force-pushed
the
2025-03-inv-server-prefactor
branch
2 times, most recently
from
feb9a8f to
0b7f7d8
Compare
|
Really clean refactor! There's a typo in the third commit message of "and and," but otherwise looks really good. |
|
🔔 1st Reminder Hey @arik-so! This PR has been waiting for your review. |
|
@jkczyz would you mind taking a look a the last commit since you requested the refactor? Thank you! |
arik-so
previously approved these changes
Mar 17, 2025
jkczyz
reviewed
Mar 18, 2025
valentinewallace
force-pushed
the
2025-03-inv-server-prefactor
branch
from
0b7f7d8 to
bd3ce45
Compare
|
Looks like there are some silent rebase conflicts causing CI to fail, will rebase now |
We need to add a Vec to the config as part of async receive support, see upcoming commits. The vec will contain blinded paths to reach an always-online node that will serve static invoices on our behalf. None of those things implement Copy so remove support for the trait.
valentinewallace
force-pushed
the
2025-03-inv-server-prefactor
branch
3 times, most recently
from
95f934d to
4a7d7e3
Compare
|
Pushed a super trivial fix for rustfmt |
jkczyz
reviewed
Mar 26, 2025
Previously when reading a BOLT 12 message from bytes, if it resulted in a DecodeError then that error would be overridden by DecodeError::InvalidValue specifically. Better to return to underlying specific error.
For context, many of our higher level ser macros such as impl_writeable_msg
support the syntax:
impl_writeable_msg!(StructName, { .. }, {
(42, tlv_field, (option, encoding: (Vec<u8>, WithoutLength>),
})
i.e. specifying an encoding wrapper around an optional field being serialized.
Here we add add similar support for an encoding wrapper around required_vecs
that are being serialized, eg (42, tlv_field, (required_vec: (encoding: (..)))).
One difficulty here is that some ser macros such as impl_writeable_msg and tlv_stream
will call the underlying macro _encode_tlv_stream with each struct field passed in as
a reference, whereas others such as impl_writeable_tlv_based will call _encode_tlv_stream
with each struct field passed in as a value.
This difference led to compilation issues because the WithoutLength ser wrapper
expects to wrap a reference to a Vec, not a value.
As a result, here we also modify the macros that were previously passing in
struct values as values to pass in references instead.
The new utility will be used in upcoming commits as part of supporting static invoice server.
Previously the compiler would not enforce that a peeled onion message had a corresponding context that matched, e.g. an offers message needs an offers blinded path context. Refactor PeeledOnion to have multiple Receive message types such that this is now enforced.
Prior to this commit we would allow DNSSEDQuery onion messages that were sent over blinded paths that were created for other purposes. This could be used to correlate identities and unblind a path, so disallow this. For consistency we also add a log on receipt of a DNSSECProof message with a missing context.
valentinewallace
force-pushed
the
2025-03-inv-server-prefactor
branch
from
4a7d7e3 to
92f0718
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.
Prepares for #3618, some commits reference commits in that upcoming PR.