-
Notifications
You must be signed in to change notification settings - Fork 887
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
close: Print multiple txs; Fixes #6467 #7466
base: master
Are you sure you want to change the base?
Conversation
42b6d65
to
2c3d916
Compare
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.
Can you deprecate the txid
field in close
?
lightningd/closing_control.c
Outdated
struct json_stream *result = json_stream_success(cc->cmd); | ||
const struct bitcoin_tx *close_tx = close_txs[tal_count(close_txs) - 1]; | ||
|
||
json_add_tx(result, "tx", close_tx); | ||
if (!invalid_last_tx(close_tx)) { | ||
struct bitcoin_txid txid; | ||
bitcoin_txid(close_tx, &txid); | ||
json_add_txid(result, "txid", &txid); |
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.
nit: deprecate the txid
field, in favor of just using the txids
field in the future.
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 deprecated txid
and tx
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 ok, missed it the first time! seems like the deprecation feature stuff got updated, very cool.
|
||
json_array_start(result, "txids"); | ||
for (int i = 0; i < tal_count(close_txs); i++) { | ||
if (invalid_last_tx(close_txs[i])) { |
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.
Man this is an old bugfix (v0.7.0)
the test failure is from one of the new fetchinvoice/offers tests |
2c3d916
to
778aae8
Compare
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.
ACK 778aae8
Changelog-Changed: `close` now outputs txs & txids of all closing transactions (splice candidates can cause there to be multiple).
Rebased, but also deprecated properly, see the second commit.
You'll note this broke tests, too. This is important; if we break an API we should absolutely feel the pain ourselves! |
778aae8
to
94705ed
Compare
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: `close` `tx` and `txid` field (use `txs` and `txids`) Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
94705ed
to
9bbada6
Compare
close
now outputs txs & txids of all closing transactions (splice candidates can cause there to be multiple).