search

bitcoinjs / bolt11

A library for encoding and decoding lightning network payment requests as defined in BOLT #11.
MIT License
93 stars 65 forks source link

complete is read before set in encode #24

Closed riperk closed 3 years ago

riperk commented 5 years ago

In the encode function we have:

https://github.com/bitcoinjs/bolt11/blob/36cbbedfd64b2d76f7242d0b9c4a827b70817395/payreq.js#L711

but data.complete is only set after:

https://github.com/bitcoinjs/bolt11/blob/36cbbedfd64b2d76f7242d0b9c4a827b70817395/payreq.js#L714

junderw commented 5 years ago

Could you provide a use case where this would cause a problem?

I guess we could remove the line all together, since complete is set in decode and sign.

A person encoding from a parsed object shouldn't have a signature, and a parsed object with a signature will get a paymentRequest and complete anyways.

riperk commented 5 years ago

I don't think it's an issue if encode is documented with the limitation that the input needs to come from the output of decode. But for people constructing the input themselves, complete acts as an (undocumented) flag that controls if paymentRequest is set to a meaningful value or not.

junderw commented 5 years ago

tbh, this was left in from before encode and sign were split.

might as well just assume no one will ever pass a signature into encode.

I might just get rid of it, since you would need a library that understands the encoding in order to create an object with the signature, and you would need to explicitly delete the complete flag to trigger this anyways.

I'll try removing it and bump major sometime this week.

junderw commented 3 years ago

I just switched the assigning order. So now passing complete to encode really does nothing, it is only based on whether we can reconstruct the signature.