Merge multifundchannel and fundchannel plugins, make fundchannel a thin wrapper around multifundchannel #3769
Conversation
ZmnSCPxj
force-pushed
the
multi-fundchannel-merge
branch
from
119acbb
to
5ee7118
Compare
|
Update based on new #3763 |
ZmnSCPxj
force-pushed
the
multi-fundchannel-merge
branch
from
5ee7118
to
f201927
Compare
|
Updated based on latest #3763 |
ZmnSCPxj
force-pushed
the
multi-fundchannel-merge
branch
3 times, most recently
from
a1ff840
to
3be3119
Compare
|
Blocked by #3763 |
cdecker
added
the
state::blocked
ZmnSCPxj
force-pushed
the
multi-fundchannel-merge
branch
from
3be3119
to
e4e5415
Compare
ZmnSCPxj
removed
the
state::blocked
|
Rebased, also reimplemented the wrapper. |
ZmnSCPxj
force-pushed
the
multi-fundchannel-merge
branch
9 times, most recently
from
ee49371
to
f806c47
Compare
| # funding tx. | ||
| addr = l1.rpc.call('dev-listaddrs', [keyidx + 3])['addresses'][-1] | ||
| addr = l1.rpc.call('dev-listaddrs', [keyidx + 2])['addresses'][-1] | ||
| # the above used to be keyidx + 3, but that was when `fundchannel` |
There was a problem hiding this comment.
this comment is unnecessary, as the commit history will preserve the deleted comment + value.
| channel_id = ok ? json_get_member(buf, channel_ids_obj, "channel_id") : NULL; | ||
| ok = ok && channel_id; | ||
| outnum = ok ? json_get_member(buf, channel_ids_obj, "outnum") : NULL; | ||
| ok = ok && outnum; |
There was a problem hiding this comment.
i hate this. just return an error at the point which it fails.
|
|
||
| if (!ok) | ||
| plugin_err(cmd->plugin, | ||
| "Unexpected result from nultifundchannel: %.*s", |
There was a problem hiding this comment.
| "Unexpected result from nultifundchannel: %.*s", | |
| "Unexpected result from multifundchannel: %.*s", |
|
cool, nice to see the code get consolidated |
There was a problem hiding this comment.
Other than minor cleanup suggested by @niftynei looks good.
| multifundchannel | ||
| pay | ||
| spenderp |
common/json.c
Outdated
| @@ -838,7 +838,12 @@ void json_add_tok(struct json_stream *result, const char *fieldname, | |||
| if (json_tok_is_num(buffer, tok)) { | |||
| json_to_int(buffer, tok, &i); | |||
| json_add_num(result, fieldname, i); | |||
| } | |||
| } else if (buffer[tok->start] == 't' || buffer[tok->start] == 'T') | |||
There was a problem hiding this comment.
Garbage in, garbage out.
If they feed in an invalid primitive, we should either pass it through, or get upset with the passer.
But I can't understand why json_add_tok exists. Why are we trying to normalize tokens? We should just append them as we're asked to.
There was a problem hiding this comment.
But I can't understand why json_add_tok exists. Why are we trying to normalize tokens? We should just append them as we're asked to.
Using json_add_member ? I don't recall exactly but it seems cleaner to use our already-present helpers.
Anyway, i 1) don't know how comes i only handled the num primitive and 2) am pretty sure Null False and True here and below are invalid JSON (maybe just use json_add_literal in the else branch).
There was a problem hiding this comment.
It also turns strings into bools, etc. It's just weird.
I'm testing the simplest variant now:
void json_add_tok(struct json_stream *result, const char *fieldname,
const jsmntok_t *tok, const char *buffer)
{
char *space;
assert(tok->type != JSMN_UNDEFINED);
space = json_member_direct(result, fieldname, json_tok_full_len(tok));
memcpy(space, json_tok_full(buffer, tok), json_tok_full_len(tok));
}I'm not sure why we were turning strings into bools, etc. Signed-off-by: Rusty Russell <[email protected]>
…ultifundchannel. Changelog-Changed: protocol: `fundchannel` now shuffles inputs and outputs, and no longer follows BIP69.
rustyrussell
force-pushed
the
multi-fundchannel-merge
branch
from
f806c47
to
ce6f30b
Compare
|
Fairly simple rebase. Removed the json_add_tok nonsense in favor of a simple copy. |
Requres #3763