Fix build for cfg(async_payments)
#3544
Conversation
lightning/src/offers/invoice.rs
Outdated
| $self: ident, $self_type: ty, $invoice_fields: expr, $return_type: ty, $return_value: expr | ||
| $(, $self_mut: tt)? |
There was a problem hiding this comment.
I'm still checking that these inscrutable arguments make sense for the new macro, but it seemed good enough for a test-changes-only PR...
valentinewallace
force-pushed
the
2025-01-fix-build
branch
from
b8e5f4c to
f3f6f4c
Compare
lightning/src/offers/invoice.rs
Outdated
| @@ -408,7 +424,8 @@ impl<'a> InvoiceWithExplicitSigningPubkeyBuilder<'a> { | |||
| invoice_explicit_signing_pubkey_builder_methods!(self, &mut Self); | |||
| invoice_builder_methods!(self, &mut Self, &mut Self, self, ExplicitSigningPubkey); | |||
| invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, Bolt12Invoice); | |||
| invoice_builder_methods_test!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self); | |||
| invoice_builder_methods_test!(self, Self, Self, self, mut); | |||
There was a problem hiding this comment.
I don't think this actually makes sense in bindings cause it requires move semantics, which most other languages don't have. Still, the allow(dead_code) screams at us..maybe we just drop the macro for this method and write it explicitly in the impl if its not used in c_bindings?
There was a problem hiding this comment.
Ah I think that might've been a copy and paste error, in the latest push I removed the move semantics. But I wasn't able to repro the allow(dead_code) warning, what command were you using to hit that?
There was a problem hiding this comment.
Ah, I was going off the #[cfg_attr(c_bindings, allow(dead_code))] on the method (which apparently should be removed).
There was a problem hiding this comment.
Looks like you're right and it can be removed, here and possibly elsewhere (although I'm not 100% sure my local testing is matching all of CI's checks). I'll go ahead and remove it if there's any further feedback, otherwise seems nicer to not delay CI further.
There was a problem hiding this comment.
IIRC, allow(dead_code) comes from the fact that with c_bindings there are two different builders (InvoiceWithExplicitSigningPubkeyBuilder and InvoiceWithDerivedSigningPubkeyBuilder). Some methods are only called on one of the two, though it's possible the surpression might be unnecessary in some cases.
Static invoices don't have an amount_msats field.
valentinewallace
force-pushed
the
2025-01-fix-build
branch
from
f3f6f4c to
4e3f8d7
Compare
|
CI passes for me locally. |
|
Backported in #3536. |
Static invoices don't have an amount_msats field.