-
Notifications
You must be signed in to change notification settings - Fork 411
Hide InvoiceFeatures behind InvoiceBuilder API #901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1ec0232
7310e26
b5f0eba
20e776b
0592c52
2226ae2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,7 +168,7 @@ pub fn check_platform() { | |
/// | ||
/// (C-not exported) as we likely need to manually select one set of boolean type parameters. | ||
#[derive(Eq, PartialEq, Debug, Clone)] | ||
pub struct InvoiceBuilder<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> { | ||
pub struct InvoiceBuilder<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> { | ||
currency: Currency, | ||
amount: Option<u64>, | ||
si_prefix: Option<SiPrefix>, | ||
|
@@ -180,6 +180,7 @@ pub struct InvoiceBuilder<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> { | |
phantom_h: std::marker::PhantomData<H>, | ||
phantom_t: std::marker::PhantomData<T>, | ||
phantom_c: std::marker::PhantomData<C>, | ||
phantom_s: std::marker::PhantomData<S>, | ||
} | ||
|
||
/// Represents a syntactically and semantically correct lightning BOLT11 invoice. | ||
|
@@ -427,7 +428,7 @@ pub mod constants { | |
pub const TAG_FEATURES: u8 = 5; | ||
} | ||
|
||
impl InvoiceBuilder<tb::False, tb::False, tb::False, tb::False> { | ||
impl InvoiceBuilder<tb::False, tb::False, tb::False, tb::False, tb::False> { | ||
/// Construct new, empty `InvoiceBuilder`. All necessary fields have to be filled first before | ||
/// `InvoiceBuilder::build(self)` becomes available. | ||
pub fn new(currrency: Currency) -> Self { | ||
|
@@ -443,14 +444,15 @@ impl InvoiceBuilder<tb::False, tb::False, tb::False, tb::False> { | |
phantom_h: std::marker::PhantomData, | ||
phantom_t: std::marker::PhantomData, | ||
phantom_c: std::marker::PhantomData, | ||
phantom_s: std::marker::PhantomData, | ||
} | ||
} | ||
} | ||
|
||
impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, C> { | ||
impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, T, C, S> { | ||
/// Helper function to set the completeness flags. | ||
fn set_flags<DN: tb::Bool, HN: tb::Bool, TN: tb::Bool, CN: tb::Bool>(self) -> InvoiceBuilder<DN, HN, TN, CN> { | ||
InvoiceBuilder::<DN, HN, TN, CN> { | ||
fn set_flags<DN: tb::Bool, HN: tb::Bool, TN: tb::Bool, CN: tb::Bool, SN: tb::Bool>(self) -> InvoiceBuilder<DN, HN, TN, CN, SN> { | ||
InvoiceBuilder::<DN, HN, TN, CN, SN> { | ||
currency: self.currency, | ||
amount: self.amount, | ||
si_prefix: self.si_prefix, | ||
|
@@ -462,6 +464,7 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, | |
phantom_h: std::marker::PhantomData, | ||
phantom_t: std::marker::PhantomData, | ||
phantom_c: std::marker::PhantomData, | ||
phantom_s: std::marker::PhantomData, | ||
} | ||
} | ||
|
||
|
@@ -482,12 +485,6 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, | |
self | ||
} | ||
|
||
/// Sets the payment secret | ||
pub fn payment_secret(mut self, payment_secret: PaymentSecret) -> Self { | ||
self.tagged_fields.push(TaggedField::PaymentSecret(payment_secret)); | ||
self | ||
} | ||
|
||
/// Sets the expiry time | ||
pub fn expiry_time(mut self, expiry_time: Duration) -> Self { | ||
match ExpiryTime::from_duration(expiry_time) { | ||
|
@@ -511,16 +508,9 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, | |
} | ||
self | ||
} | ||
|
||
/// Adds a features field which indicates the set of supported protocol extensions which the | ||
/// origin node supports. | ||
pub fn features(mut self, features: InvoiceFeatures) -> Self { | ||
self.tagged_fields.push(TaggedField::Features(features)); | ||
self | ||
} | ||
} | ||
|
||
impl<D: tb::Bool, H: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, tb::True, C> { | ||
impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, tb::True, C, S> { | ||
/// Builds a `RawInvoice` if no `CreationError` occurred while construction any of the fields. | ||
pub fn build_raw(self) -> Result<RawInvoice, CreationError> { | ||
|
||
|
@@ -553,9 +543,9 @@ impl<D: tb::Bool, H: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, tb::True, C> { | |
} | ||
} | ||
|
||
impl<H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<tb::False, H, T, C> { | ||
impl<H: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<tb::False, H, T, C, S> { | ||
/// Set the description. This function is only available if no description (hash) was set. | ||
pub fn description(mut self, description: String) -> InvoiceBuilder<tb::True, H, T, C> { | ||
pub fn description(mut self, description: String) -> InvoiceBuilder<tb::True, H, T, C, S> { | ||
match Description::new(description) { | ||
Ok(d) => self.tagged_fields.push(TaggedField::Description(d)), | ||
Err(e) => self.error = Some(e), | ||
|
@@ -564,23 +554,23 @@ impl<H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<tb::False, H, T, C> { | |
} | ||
|
||
/// Set the description hash. This function is only available if no description (hash) was set. | ||
pub fn description_hash(mut self, description_hash: sha256::Hash) -> InvoiceBuilder<tb::True, H, T, C> { | ||
pub fn description_hash(mut self, description_hash: sha256::Hash) -> InvoiceBuilder<tb::True, H, T, C, S> { | ||
self.tagged_fields.push(TaggedField::DescriptionHash(Sha256(description_hash))); | ||
self.set_flags() | ||
} | ||
} | ||
|
||
impl<D: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, tb::False, T, C> { | ||
impl<D: tb::Bool, T: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, tb::False, T, C, S> { | ||
/// Set the payment hash. This function is only available if no payment hash was set. | ||
pub fn payment_hash(mut self, hash: sha256::Hash) -> InvoiceBuilder<D, tb::True, T, C> { | ||
pub fn payment_hash(mut self, hash: sha256::Hash) -> InvoiceBuilder<D, tb::True, T, C, S> { | ||
self.tagged_fields.push(TaggedField::PaymentHash(Sha256(hash))); | ||
self.set_flags() | ||
} | ||
} | ||
|
||
impl<D: tb::Bool, H: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, tb::False, C> { | ||
impl<D: tb::Bool, H: tb::Bool, C: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, tb::False, C, S> { | ||
/// Sets the timestamp. | ||
pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder<D, H, tb::True, C> { | ||
pub fn timestamp(mut self, time: SystemTime) -> InvoiceBuilder<D, H, tb::True, C, S> { | ||
match PositiveTimestamp::from_system_time(time) { | ||
Ok(t) => self.timestamp = Some(t), | ||
Err(e) => self.error = Some(e), | ||
|
@@ -590,22 +580,48 @@ impl<D: tb::Bool, H: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, tb::False, C> { | |
} | ||
|
||
/// Sets the timestamp to the current UNIX timestamp. | ||
pub fn current_timestamp(mut self) -> InvoiceBuilder<D, H, tb::True, C> { | ||
pub fn current_timestamp(mut self) -> InvoiceBuilder<D, H, tb::True, C, S> { | ||
let now = PositiveTimestamp::from_system_time(SystemTime::now()); | ||
self.timestamp = Some(now.expect("for the foreseeable future this shouldn't happen")); | ||
self.set_flags() | ||
} | ||
} | ||
|
||
impl<D: tb::Bool, H: tb::Bool, T: tb::Bool> InvoiceBuilder<D, H, T, tb::False> { | ||
impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, S: tb::Bool> InvoiceBuilder<D, H, T, tb::False, S> { | ||
/// Sets `min_final_cltv_expiry`. | ||
pub fn min_final_cltv_expiry(mut self, min_final_cltv_expiry: u64) -> InvoiceBuilder<D, H, T, tb::True> { | ||
pub fn min_final_cltv_expiry(mut self, min_final_cltv_expiry: u64) -> InvoiceBuilder<D, H, T, tb::True, S> { | ||
self.tagged_fields.push(TaggedField::MinFinalCltvExpiry(MinFinalCltvExpiry(min_final_cltv_expiry))); | ||
self.set_flags() | ||
} | ||
} | ||
|
||
impl InvoiceBuilder<tb::True, tb::True, tb::True, tb::True> { | ||
impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, C, tb::False> { | ||
/// Sets the payment secret and relevant features. | ||
pub fn payment_secret(mut self, payment_secret: PaymentSecret) -> InvoiceBuilder<D, H, T, C, tb::True> { | ||
let features = InvoiceFeatures::empty() | ||
.set_variable_length_onion_required() | ||
.set_payment_secret_required(); | ||
self.tagged_fields.push(TaggedField::PaymentSecret(payment_secret)); | ||
self.tagged_fields.push(TaggedField::Features(features)); | ||
self.set_flags() | ||
} | ||
} | ||
|
||
impl<D: tb::Bool, H: tb::Bool, T: tb::Bool, C: tb::Bool> InvoiceBuilder<D, H, T, C, tb::True> { | ||
/// Sets the `basic_mpp` feature as optional. | ||
pub fn basic_mpp(mut self) -> Self { | ||
valentinewallace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.tagged_fields = self.tagged_fields | ||
.drain(..) | ||
.map(|field| match field { | ||
TaggedField::Features(f) => TaggedField::Features(f.set_basic_mpp_optional()), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note, what if in the future we introduce a new tagged field requiring to set the Maybe we should add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is conditionally implemented when |
||
_ => field, | ||
}) | ||
.collect(); | ||
self | ||
} | ||
} | ||
|
||
impl<S: tb::Bool> InvoiceBuilder<tb::True, tb::True, tb::True, tb::True, S> { | ||
/// Builds and signs an invoice using the supplied `sign_function`. This function MAY NOT fail | ||
/// and MUST produce a recoverable signature valid for the given hash and if applicable also for | ||
/// the included payee public key. | ||
|
@@ -644,6 +660,7 @@ impl InvoiceBuilder<tb::True, tb::True, tb::True, tb::True> { | |
}; | ||
|
||
invoice.check_field_counts().expect("should be ensured by type signature of builder"); | ||
invoice.check_feature_bits().expect("should be ensured by type signature of builder"); | ||
|
||
Ok(invoice) | ||
} | ||
|
@@ -972,6 +989,41 @@ impl Invoice { | |
Ok(()) | ||
} | ||
|
||
/// Check that feature bits are set as required | ||
fn check_feature_bits(&self) -> Result<(), SemanticError> { | ||
// "If the payment_secret feature is set, MUST include exactly one s field." | ||
let payment_secret_count = self.tagged_fields().filter(|&tf| match *tf { | ||
TaggedField::PaymentSecret(_) => true, | ||
_ => false, | ||
}).count(); | ||
if payment_secret_count > 1 { | ||
return Err(SemanticError::MultiplePaymentSecrets); | ||
} | ||
|
||
// "A writer MUST set an s field if and only if the payment_secret feature is set." | ||
let has_payment_secret = payment_secret_count == 1; | ||
let features = self.tagged_fields().find(|&tf| match *tf { | ||
TaggedField::Features(_) => true, | ||
_ => false, | ||
}); | ||
match features { | ||
None if has_payment_secret => Err(SemanticError::InvalidFeatures), | ||
None => Ok(()), | ||
Some(TaggedField::Features(features)) => { | ||
if features.supports_payment_secret() && has_payment_secret { | ||
Ok(()) | ||
} else if has_payment_secret { | ||
Err(SemanticError::InvalidFeatures) | ||
} else if features.supports_payment_secret() { | ||
Err(SemanticError::InvalidFeatures) | ||
} else { | ||
Ok(()) | ||
} | ||
}, | ||
Some(_) => unreachable!(), | ||
} | ||
} | ||
|
||
/// Check that the invoice is signed correctly and that key recovery works | ||
pub fn check_signature(&self) -> Result<(), SemanticError> { | ||
match self.signed_invoice.recover_payee_pub_key() { | ||
|
@@ -1006,6 +1058,7 @@ impl Invoice { | |
signed_invoice: signed_invoice, | ||
}; | ||
invoice.check_field_counts()?; | ||
invoice.check_feature_bits()?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there cases where you may want to use this on an invoice not generated by us? Where we may want to accept a non-basic-mpp invoice but not generate them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is supported as written. |
||
invoice.check_signature()?; | ||
|
||
Ok(invoice) | ||
|
@@ -1298,6 +1351,12 @@ pub enum SemanticError { | |
/// The invoice contains multiple descriptions and/or description hashes which isn't allowed | ||
MultipleDescriptions, | ||
|
||
/// The invoice contains multiple payment secrets | ||
MultiplePaymentSecrets, | ||
|
||
/// The invoice's features are invalid | ||
InvalidFeatures, | ||
|
||
/// The recovery id doesn't fit the signature/pub key | ||
InvalidRecoveryId, | ||
|
||
|
@@ -1312,6 +1371,8 @@ impl Display for SemanticError { | |
SemanticError::MultiplePaymentHashes => f.write_str("The invoice has multiple payment hashes which isn't allowed"), | ||
SemanticError::NoDescription => f.write_str("No description or description hash are part of the invoice"), | ||
SemanticError::MultipleDescriptions => f.write_str("The invoice contains multiple descriptions and/or description hashes which isn't allowed"), | ||
SemanticError::MultiplePaymentSecrets => f.write_str("The invoice contains multiple payment secrets"), | ||
SemanticError::InvalidFeatures => f.write_str("The invoice's features are invalid"), | ||
SemanticError::InvalidRecoveryId => f.write_str("The recovery id doesn't fit the signature/pub key"), | ||
SemanticError::InvalidSignature => f.write_str("The invoice's signature is invalid"), | ||
} | ||
|
@@ -1464,6 +1525,97 @@ mod test { | |
assert!(new_signed.check_signature()); | ||
} | ||
|
||
#[test] | ||
fn test_check_feature_bits() { | ||
use TaggedField::*; | ||
use lightning::ln::features::InvoiceFeatures; | ||
use secp256k1::Secp256k1; | ||
use secp256k1::key::SecretKey; | ||
use {RawInvoice, RawHrp, RawDataPart, Currency, Sha256, PositiveTimestamp, Invoice, | ||
SemanticError}; | ||
|
||
let private_key = SecretKey::from_slice(&[42; 32]).unwrap(); | ||
let payment_secret = lightning::ln::PaymentSecret([21; 32]); | ||
let invoice_template = RawInvoice { | ||
hrp: RawHrp { | ||
currency: Currency::Bitcoin, | ||
raw_amount: None, | ||
si_prefix: None, | ||
}, | ||
data: RawDataPart { | ||
timestamp: PositiveTimestamp::from_unix_timestamp(1496314658).unwrap(), | ||
tagged_fields: vec ! [ | ||
PaymentHash(Sha256(sha256::Hash::from_hex( | ||
"0001020304050607080900010203040506070809000102030405060708090102" | ||
).unwrap())).into(), | ||
Description( | ||
::Description::new( | ||
"Please consider supporting this project".to_owned() | ||
).unwrap() | ||
).into(), | ||
], | ||
}, | ||
}; | ||
|
||
// Missing features | ||
let invoice = { | ||
let mut invoice = invoice_template.clone(); | ||
invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); | ||
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) | ||
}.unwrap(); | ||
assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::InvalidFeatures)); | ||
|
||
// Missing feature bits | ||
let invoice = { | ||
let mut invoice = invoice_template.clone(); | ||
invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); | ||
invoice.data.tagged_fields.push(Features(InvoiceFeatures::empty()).into()); | ||
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) | ||
}.unwrap(); | ||
assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::InvalidFeatures)); | ||
|
||
// Including payment secret and feature bits | ||
let invoice = { | ||
let mut invoice = invoice_template.clone(); | ||
invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); | ||
invoice.data.tagged_fields.push(Features(InvoiceFeatures::known()).into()); | ||
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) | ||
}.unwrap(); | ||
assert!(Invoice::from_signed(invoice).is_ok()); | ||
|
||
// No payment secret or features | ||
let invoice = { | ||
let invoice = invoice_template.clone(); | ||
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) | ||
}.unwrap(); | ||
assert!(Invoice::from_signed(invoice).is_ok()); | ||
|
||
// No payment secret or feature bits | ||
let invoice = { | ||
let mut invoice = invoice_template.clone(); | ||
invoice.data.tagged_fields.push(Features(InvoiceFeatures::empty()).into()); | ||
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) | ||
}.unwrap(); | ||
assert!(Invoice::from_signed(invoice).is_ok()); | ||
|
||
// Missing payment secret | ||
let invoice = { | ||
let mut invoice = invoice_template.clone(); | ||
invoice.data.tagged_fields.push(Features(InvoiceFeatures::known()).into()); | ||
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) | ||
}.unwrap(); | ||
assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::InvalidFeatures)); | ||
|
||
// Multiple payment secrets | ||
let invoice = { | ||
let mut invoice = invoice_template.clone(); | ||
invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); | ||
invoice.data.tagged_fields.push(PaymentSecret(payment_secret).into()); | ||
invoice.sign::<_, ()>(|hash| Ok(Secp256k1::new().sign_recoverable(hash, &private_key))) | ||
}.unwrap(); | ||
assert_eq!(Invoice::from_signed(invoice), Err(SemanticError::MultiplePaymentSecrets)); | ||
} | ||
|
||
#[test] | ||
fn test_builder_amount() { | ||
use ::*; | ||
|
@@ -1621,14 +1773,16 @@ mod test { | |
.route(route_1.clone()) | ||
.route(route_2.clone()) | ||
.description_hash(sha256::Hash::from_slice(&[3;32][..]).unwrap()) | ||
.payment_hash(sha256::Hash::from_slice(&[21;32][..]).unwrap()); | ||
.payment_hash(sha256::Hash::from_slice(&[21;32][..]).unwrap()) | ||
.payment_secret(PaymentSecret([42; 32])) | ||
.basic_mpp(); | ||
|
||
let invoice = builder.clone().build_signed(|hash| { | ||
secp_ctx.sign_recoverable(hash, &private_key) | ||
}).unwrap(); | ||
|
||
assert!(invoice.check_signature().is_ok()); | ||
assert_eq!(invoice.tagged_fields().count(), 8); | ||
assert_eq!(invoice.tagged_fields().count(), 10); | ||
|
||
assert_eq!(invoice.amount_pico_btc(), Some(123)); | ||
assert_eq!(invoice.currency(), Currency::BitcoinTestnet); | ||
|
@@ -1646,6 +1800,8 @@ mod test { | |
InvoiceDescription::Hash(&Sha256(sha256::Hash::from_slice(&[3;32][..]).unwrap())) | ||
); | ||
assert_eq!(invoice.payment_hash(), &sha256::Hash::from_slice(&[21;32][..]).unwrap()); | ||
assert_eq!(invoice.payment_secret(), Some(&PaymentSecret([42; 32]))); | ||
assert_eq!(invoice.features(), Some(&InvoiceFeatures::known())); | ||
|
||
let raw_invoice = builder.build_raw().unwrap(); | ||
assert_eq!(raw_invoice, *invoice.into_signed_raw().raw_invoice()) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, we still need to be able to set more features here, no? You may want to set some future features which is probably in
known
but not in the set that is required to understand the invoice here.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean setting a feature as optional? If so, that is an open question that I had. For instance, would we want to have both
basic_mpp_required
andbasic_mpp_optional
methods? It wasn't entirely clear from the BOLT if either were possible:Does "offered" mean the odd bit (optional) was set? Is something similar possible for
payment_secret
? The BOLT seems to imply that if a secret is set then it must be used:And what would optional mean here if
basic_mpp
is set as required as there is a dependency requirement?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant a feature other than the two below. eg what if we have
option_htlcs_colored_blue
? Some users may really like blue HTLCs some users may not, and LDK may reasonably support receiving both. Thus, we need some way to provide aFeatures
object, I think.I believe it means either. I think the handling of var_len_onion here is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so the additional type parameter
S
is for specifying that a secret can be set at most once. Additionally, it is used to condition whetherbasic_mpp
can bet set because that requires thepayment_secret
feature and thus also a secret field set in the invoice.Other features may have similar type parameters associated with them if they require some other fields set in the invoice. For example, if the hypothetical feature were instead
option_htlcs_colored
with an additional field specifying the color blue, then we would add anoption_htlcs_colored
method taking a color to set in the invoice. It would also set the feature bits as necessary.Long story short is the purpose of
InvoiceBuilder
is to enforce BOLT 11 semantics at compile time. Thus, it can never result in aSemanticsError
only aCreationError
. Allowing features to be set arbitrarily breaks that contract.Whether the features are set as required or optional is an orthogonal concern. Since these features are optional in
InvoiceFeatures::known()
, then we'd likely want to set them optional here. I think I had decided on required because the dependency between them and wasn't sure what it would mean for some to be optional and others to be required.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmmmm, right, for "known" bits that makes sense. I do wonder how we can support "experimental" feature bits (eg DLC invoices or so), but we really need the ability to set them in
InvoiceFeatures
as well which would imply its a separate thing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For experimental features, I'd imagine there could be a separate method to set these which could also check against any known features.