Offers fixes #4669
Merged
Offers fixes #4669
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
@shesek points out that we called this field created_at in bolt11 decode, which makes more sense anyway. Changelog-EXPERIMENTAL: bolt12 decode `timestamp` field deprecated in favor of new name `created_at`. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
You could have quantity_min of 0, which makes no sense; spec has been updated, so quote and enforce that. Reported-by: @shesek Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…d it. We don't automatically *reject* an invoice which asks for a different msat than we specified (caller may!), but we don't bother noting it unless it is different. Reported-by: @shesek Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Turns out we didn't actually test this at all, and next commit does :(
offer_status_in_db: 4 is invalid
lightningd: FATAL SIGNAL 6 (version v0.10.0-459-g48fbd45-modded)
0x5608cd360855 send_backtrace
common/daemon.c:39
0x5608cd3608ff crashdump
common/daemon.c:52
0x7f9af1dae20f ???
???:0
0x7f9af1dae18b ???
???:0
0x7f9af1d8d858 ???
???:0
0x5608cd30a47e fatal
lightningd/log.c:819
0x5608cd3430c5 offer_status_in_db
wallet/wallet.h:1424
0x5608cd34f1f3 wallet_offer_disable
wallet/wallet.c:4494
0x5608cd33ae2e json_disableoffer
lightningd/offer.c:256
0x5608cd3038fc command_exec
lightningd/jsonrpc.c:643
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As requested by @shesek: it's weird to fail if they ask for the exact same thing (which is quite possible, since offers don't expire by default). And add a new "created" field so they can tell if they have an old one. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was a bug in the spec, actually. Reported-by: @shesek Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Fixes: ElementsProject#4666
In particular, Shesek tried an empty offer description, and the resulting signature didn't match since it was omitted entirely from the bolt12 string! Reported-by: @shesek Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. We assumed an empty upfront_shutdown_script TLV would become NULL:
RPC call failed: method: fundchannel, payload: {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'amount': 1000000, 'announce': True}, error: {'code': -1, 'message': 'They sent error channel e7c2d5d14462fe269631418fbfc3db327843382e6a2a5a9c2991d2d6ba31d9f5: Unacceptable upfront_shutdown_script ', 'data': {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'method': 'fundchannel_start'}}"
2. We were assuming an empty enctlv would become NULL, too.
We should not have done this (there's a semantic difference between
"empty" and not-present for TLVs), so prepare for the change.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#4667 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
7b26f28 to
2f02994
Compare
vincenzopalazzo
approved these changes
Jul 21, 2021
There was a problem hiding this comment.
ack 2f02994
Wow, enough code to digest for today, I hope I didn't miss nothings :)
niftynei
approved these changes
Jul 21, 2021
| &active)) { | ||
| return command_fail(cmd, | ||
| LIGHTNINGD, | ||
| "Bad creaoffer status reply %.*s", |
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.
Mainly due to great feedback from @shesek !