json_tok* is used with 'struct command', so rename this to match the other
low-level json tok helpers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This currently just invokes GDB, but we could generalize it (though
pdb doesn't allow attaching to a running process, other python
debuggers seem to).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is prep work for when we sign htlc txs with
SIGHASH_SINGLE|SIGHASH_ANYONECANPAY.
We still deal with raw signatures for the htlc txs at the moment, since
we send them like that across the wire, and changing that was simply too
painful (for the moment?).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We currently generally assume the features we offer are fixed; this
makes the code clearer and handles where we offer features iff
EXPERIMENTAL_FEATURES=1
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Needed for check command. I left the print function in since it was so
convenient for debugging purposes.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
This simplifies lifetime assumptions. Currently all callers keep the
original around, but everything broke when I changed that in the next
patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They were not universally used, and most are trivial accessors anyway.
The exception is getting the channel reserve: we have to multiply by 1000
as well as flip direction, so keep that one.
The BOLT quotes move to `struct channel_config`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We only take the pubkey and ignore all other fields, so we might as well
save the cycles used computing the hash for something else.
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
We probably also want to call secp_randomise/wally_secp_randomize here
too, and since these calls all call setup_tmpctx, it probably makes
sense to have a helper function to do all that. Until thats done, I
modified the tests so grepping will show the places where the sequence
of calls is repeated.
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
This avoids some very ugly switch() statements which mixed the two,
but we also take the chance to rename 'towire_gossip_' to
'towire_gossipd_' for those inter-daemon messages; they're messages to
gossipd, not gossip messages.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had at least one bug caused by it not returning true when it had
queued something. Instead, just re-check thq queue after it's called.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If you steal something onto its own child, you create a loop. These are
expensive to check for at runtime, but they can hide from memleak and are
usually a bad idea. So we add a tal_steal() notify which does this work
in DEVELOPER mdoe.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This way there's no need for a context pointer, and freeing a msg_queue
frees its contents, as expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It means an extra allocation at startup, but it means we can hide the definition,
and use standard patterns (new_daemon_conn and typesafe callbacks).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was suggested by Pierre-Marie as the solution to the 'same HTLC,
different CLTV' signature mismatch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's the only user of them, and it's going to get optimized.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossip.pydiff --git a/common/test/run-json.c b/common/test/run-json.c
index 956fdda35..db52d6b01 100644
This also highlights the danger of searching the logs: that error
appeared previously in the logs, so we didn't notice that the actual
withdraw call gave a different error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Give a clear error at the beginning if it's not bolt11 payment,
rather than falling foul of other checks.
This will work at least until some altcoin adapts the 'ln' prefix :)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Totally forgot to add this test. It just shows how a writer can take
exclusive access of a socket over multiple `io_write` calls, and
queuing all others behind it.
We've done this a number of times already where we're getting
exclusive access to either the out direction of a connection, or we
try to lock out the read side while we are responding to a previous
request. They usually are really cumbersome because we reach around to
the other direction to stop it from proceeding, or we flag our
exclusive access somewhere, and we always need to know whom to notify.
PR ElementsProject/lightning#1970 adds two new instances of this:
- Streaming a JSON response requires that nothing else should write
while the stream is active.
- We also want to stop reading new requests while we are responding
to one.
To remove the complexity of having to know whom to stop and notify
when we're done, this adds a simple `io_lock` primitive that can be
used to get exclusive access to a connection. This inverts the
requirement for notifications, since everybody registers interest in
the lock and they get notified if the lock holder releases it.
connectd is the only user of the cryptomsg async APIs; better to
open-code it here. We need to expose a little from cryptomsg(),
but we remove the 'struct peer' entirely from connectd.
One trick is that we still need to defer telling lightningd when a
peer reconnects (until it tells us the old one is disconnected). So
now we generate the message for lightningd and send it once we're woken.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We do this a lot, and had boutique helpers in various places. So add
a more generic one; for convenience it returns a pointer to the new
end element.
I prefer the name tal_arr_expand to tal_arr_append, since it's up to
the caller to populate the new array entry.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It turns out we were heavily relying on the fact that after each message from
the client there'd be a flush, and that there would not be anything after the
JSON object we read. This will no longer be the case once we start streaming
things or we are very quick in issuing the JSON-RPC requests.
This just takes one of the error paths (incomplete read) and makes it into a
successful path if we have indeed read a full root element.
That matches the other CSV names (HSM was the first, so it was written
before the pattern emerged).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
this enables addr like --addr=autotor:127.0.0.1 or
--addr=autotor:localhost to just use the default tor service port
Signed-off-by: Saibato <Saibato.naga@pm.me>
Code changes:
1. Expose daemon_poll() so lightningd can call it directly, which avoids us
having store a global and document it.
2. Remove the (undocumented, unused, forgotten) --rpc-file="" option to disable
JSON RPC.
3. Move the ickiness of finding the executable path into subd.c, so it doesn't
distract from lightningd.c overview.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to exclude the child from being entered into the htable:
if we wanted the parent we could do this outside the loop.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
memleak can't see into htables, as it overloads unused pointer bits.
And it can't see into intmap, since they use malloc (it only looks for tal
pointers).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I managed to crash the HSM by asking for point -1 (shachain_index has an
assert). Fail in this case, instead.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
To be safe, we should never memcmp secrets. We don't do this
currently outside tests, but we're about to.
The tests to prove this as constant time are the tricky bit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tests were failing when in the same thread after a test which set
log_all_io=True, because SIGUSR1 seemed to be turning logging *off*.
This is due to Python using references not copies for assignment.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is required for the next test, which has to log messages from channeld
as soon as it starts (so might be too late if it sends SIGUSR1).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We ignore incoming for now, but this means we advertize the option and
we send the required fields.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a wrapper around shachain_get_hash, which converts the
commit_num to an index and returns a 'struct secret' rather than a
'struct sha256' (which is really an internal detail).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was a very simple change and allowed us to remove the special
`json_opt_tok` macro.
Moved the callback out of `common/json.c` to `lightningd/json.c` because the new
callbacks are dependent on `struct command` etc.
(I already started on `json_tok_number`)
My plan is to:
1. upgrade json_tok_X one a time, maybe a PR for each one.
2. When done, rename macros (i.e, remove "_tal").
3. Remove all vestiges of the old callbacks
4. Add new callbacks so that we no longer need json_tok_tok!
(e.g., json_tok_label, json_tok_str, json_tok_msat)
Signed-off-by: Mark Beckwith <wythe@intrig.com>
Avoid that 200ms loss. We don't want to disable nagle generally,
since it's great for gossip and other traffic; we just want to push at
critical times.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We currently hand the error back to the master, who then stores it for
future connections and hands it back to another openingd to send and exit.
Just send directly; it's more reliable and simpler.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Also means we simplify the handle_gossip_msg() since everyone wants it to
use sync_crypto_write().
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The One Big API is confusing, and has enough corner cases that we should
ditch it rather than add more.
See: https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction
In particular, when openingd is changed to chat to peers even when
it's not actively opening a channel, it wants to handle (most) errors
by continuing, not calling peer_failed().
This exposes the constituent parts.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It no longer has any effect on tal_len(), but it *does* give file and line
of allocations which is much nicer for tracking memory leaks!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In some daemons I want to hand it into a loop, which would call
clean_tmpctx(). This causes a subtle bug.
So just free the children directly: the pointer itself remains valid.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The easiest way to do this is to play with the 'wallet_tx' semantics
and have 'amount' have meaning even when 'all_funds' is set.
Note that we change the string 'Cannot afford funding transaction' to
'Cannot afford transaction' as this code is also used for withdrawls.
Inspired-by: molz on #c-lightning
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In several places we use low-level tal functions because we want the
label to be something other than the default. ccan/tal is adding
tal_*_label so replace them and shim it for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
tal_count() is used where there's a type, even if it's char or u8, and
tal_bytelen() is going to replace tal_len() for clarity: it's only needed
where a pointer is void.
We shim tal_bytelen() for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There are three cases:
1. failcode is 0, scid is NULL, failreason is the onion to fwd.
2. failcode is non-zero, but UPDATE bit not set. scid is NULL, failreason NULL.
3. failcode has UPDATE bit set. scid is non-NULL, failreason is NULL.
Assert these on marshaling, and only send the parts we need so unmarshal is
always canonical.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The master tells us the short_channel_id of the outgoing channel when
failing an HTLC, but channeld didn't store it anywhere. It also
didn't tell channeld the short_channel_id in the case where we're
reconnecting and it's feeding us an array of failed htlcs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
That was the cause of the bad gossip order failures: gossipd thought our
channel was live, but the other end didn't receive message last time.
Now gossipd doesn't use fd to kill us (connectd tells master to do so), we
can implement read_peer_msg_nogossip().
Fixes: #1706
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Removed `json_get_params`.
Also added json_tok_percent and json_tok_newaddr. Probably should
have been a separate PR but it was so easy.
[ Squashed comment update for gcc workaround --RR ]
Signed-off-by: Mark Beckwith <wythe@intrig.com>
Often we only need a single secret, so it's clearer to have routines
to do just that. When we change to the lnd key scheme, there will be
no benefit in calculating them all together.
This also adds a test!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Well, it's generated by shachain, so technically it is a sha256, but
that's an internal detail. It's a secret.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I'm not completely convinced that it's only ever set to a failcode
with the BADONION bit set, especially after the previous patches in
this series. Now that channeld can handle arbitrary failcodes passed
this way, simply rename it.
We add marshalling assertions that only one of failcode and failreason
is set, and we unmarshal an empty 'fail' to NULL (just the the
generated unmarshalling code does).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is part of #1464 and incorporates Rusty's suggested updates from #1569.
See comment in param.h for description, here's the basics:
unsigned cltv;
const jsmntok_t *note;
u64 msatoshi;
struct param * mp;
if (!param_parse(cmd, buffer, tokens,
param_req("cltv", json_tok_number, &cltv),
param_opt("note", json_tok_tok, ¬e),
mp = param_opt("msatoshi", json_tok_u64, &msatoshi),
NULL))
return;
if (param_is_set(mp))
do_something()
There is a lot of developer mode code to make sure we don't make mistakes,
like trying to unmarshal into the same variable twice or adding a required param
after optional.
During testing, I found a bug (of sorts) in the current system. It allows you
to provide two named parameters with the same name without error; e.g.:
# cli/lightning-cli -k newaddr addresstype=p2sh-segwit addresstype=bech32
{
"address": "2N3r6fT65PhfhE1mcMS6TtcdaEurud6M7pA"
}
It just takes the first and ignores the second. The new system reports this as an
error for now. We can always change this later.
structeq() is too dangerous: if a structure has padding, it can fail
silently.
The new ccan/structeq instead provides a macro to define foo_eq(),
which does the right thing in case of padding (which none of our
structures currently have anyway).
Upgrade ccan, and use it everywhere. Except run-peer-wire.c, which
is only testing code and can use raw memcmp(): valgrind will tell us
if padding exists.
Interestingly, we still declared short_channel_id_eq, even though
we didn't define it any more!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a best effort attempt to skip connection attempts if we detect a broken
ISP resolver. A broken ISP resolver is a resolver that will replace NXDOMAIN
replies with a dummy response. This is best effort in that it'll only detect a
single fixed dummy reply, it'll check only on startup, and will not detect if we
switched networks. It should be good enough for most cases, and in the worst
case it will result in a connection attempt that does not complete.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Reported-by: Glenn Willen <@gwillen>
New codes: FUND_MAX_EXCEEDED, FUND_CANNOT_AFFORD, FUND_DUST_LIMIT_UNMET.
The error message "Cannot afford fee" was not exactly correct because
it would also occur if the amount requested could not be afforded. So
I changed it to the more generic "Cannot afford transaction".
Other things:
* Fixed off-by-one satoshi in fundchannel manpage.
* Changed 'arror' to 'error' because we are not pirates.
2018-06-14T01:09:03.495Z lightningd(23766): HSM: created new hsm_secret file
==23785== Syscall param socketcall.bind(my_addr.sin6_flowinfo) points to uninitialised byte(s)
==23785== at 0x5731877: bind (syscall-template.S:78)
==23785== by 0x11767C: make_listen_fd (gossip.c:2405)
==23785== by 0x117DA2: handle_wireaddr_listen (gossip.c:2558)
==23785== by 0x1183B7: setup_listeners (gossip.c:2653)
==23785== by 0x118E86: gossip_activate (gossip.c:2871)
==23785== by 0x11AC42: recv_req (gossip.c:3543)
==23785== by 0x143FF1: next_plan (io.c:59)
==23785== by 0x144AEE: do_plan (io.c:387)
==23785== by 0x144B2C: io_ready (io.c:397)
==23785== by 0x146719: io_loop (poll.c:310)
==23785== by 0x11B0B0: main (gossip.c:3687)
==23785== Address 0x1ffeffffa4 is on thread 1's stack
==23785== in frame #2, created by handle_wireaddr_listen (gossip.c:2539)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Until now, `command_fail()` reported an error code of -1 for all uses.
This PR adds an `int code` parameter to `command_fail()`, requiring the
caller to explicitly include the error code.
This is part of #1464.
The majority of the calls are used during parameter validation and
their error code is now JSONRPC2_INVALID_PARAMS.
The rest of the calls report an error code of LIGHTNINGD, which I defined to
-1 in `jsonrpc_errors.h`. The intention here is that as we improve our error
reporting, all occurenaces of LIGHTNINGD will go away and we can eventually
remove it.
I also converted calls to `command_fail_detailed()` that took a `NULL` `data`
parameter to use the new `command_fail()`.
The only difference from an end user perspecive is that bad input errors that
used to be -1 will now be -32602 (JSONRPC2_INVALID_PARAMS).
1. We need to test all bits, not all bytes.
2. Both local and global features need to be supported.
3. Untested code is broken code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tor wasn't actually working for me to connect to anything, but it worked
for 'ssh -D' testing.
Note that the resulting 'netaddr' is a bit weird, but I guess it's honest.
$ ./cli/lightning-cli connect 021f2cbffc4045ca2d70678ecf8ed75e488290874c9da38074f6d378248337062b
{
"id": "021f2cbffc4045ca2d70678ecf8ed75e488290874c9da38074f6d378248337062b"
}
$ ./cli/lightning-cli listpeers
{
"peers": [
{
"state": "GOSSIPING",
"id": "021f2cbffc4045ca2d70678ecf8ed75e488290874c9da38074f6d378248337062b",
"netaddr": [
"ln1qg0je0lugpzu5ttsv78vlrkhteyg9yy8fjw68qr57mfhsfyrxurzkq522ah.lseed.bitcoinstats.com:9735"
],
"connected": true,
"owner": "lightning_gossipd"
}
]
}
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is useful for the next patch, where we want to hand the unresolved
name through to the proxy.
This also addresses @Saibato's worry that we still called getaddrinfo()
(with the AI_NUMERICHOST option) even if we didn't want a lookup.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means it will effect connect commands too (though it's too
late to stop DNS lookups caused by commandline options).
We also warn that this is one case where we allow forcing through Tor
without a proxy set: it just means all connections will fail.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This takes the Tor service address in the same option, rather than using
a separate one. Gossipd now digests this like any other type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For the moment, this is a straight handing of current parameters through
from master to the gossip daemon. Next we'll change that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Currently it's always for messages to peer: make that status_peer_io and
add a new status_io for other IO.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Risks leakage. We could do lookup via the proxy, but that's a TODO.
There's only one occurance of getaddrinfo (and no gethostbyname), so
we add a flag to the callers.
Note: the use of --always-use-proxy suppresses *all* DNS lookups, even
those from connect commands and the command line.
FIXME: An implicit setting of use_proxy_always is done in gossipd if it
determines that we are announcing nothing but Tor addresses, but that
does *not* suppress 'connect'.
This is fixed in a later patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And use it in wireaddr.
We fix up the double '.onion' in the test case, which seems like an error?
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is simply the code to set up the automatic hidden service, so move
it into lightningd.
I removed the undefined parse_tor_wireaddr, and added a parameter name
to the create_tor_hidden_service_conn() declaration for update-mocks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
All gossipd needs from common/tor is do_we_use_tor_addr(), so move
that and the rest of the tor-specific handshake code into gossip/tor.c
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a rebased and combined patch for Tor support. It is extensively
reworked in the following patches, but the basis remains Saibato's work,
so it seemed fairest to begin with this.
Minor changes:
1. Use --announce-addr instead of --tor-external.
2. I also reverted some whitespace and unrelated changes from the patch.
3. Removed unnecessary ';' after } in functions.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
on all platforms;
because of that BUILD_ASSERT was failing on macOS.
(on macOS "sizeof(sun->sun_path) == 104" and
"sizeof(addr->u.sockname) == 108")
[ Linux man page says it can be as small as 92, so let's use the real value.
I also cleaned up the incorrect comment order on that struct! --RR ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Someone could try to announce an internal address, and we might probe
it.
This breaks tests, so we add '--dev-allow-localhost' for our tests, so
we don't eliminate that one. Of course, now we need to skip some more
tests in non-developer mode.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we're given a wildcard address, we can't announce it like that: we need
to try to turn it into a real address (using guess_address). Then we
use that address. As a side-effect of this cleanup, we only announce
*any* '--addr' if it's routable.
This fix means that our tests have to force '--announce-addr' because
otherwise localhost isn't routable.
This means that gossipd really controls the addresses now, and breaks
them into two arrays: what we bind to, and what we announce. That is
now what we return to the master for json_getinfo(), which prints them
as 'bindings' and 'addresses' respectively.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
Mark Beckwith
Christian Decker
arowser
Jon Griffiths
lisa neigut
Saibato
practicalswift
Mark Beckwith
Saibato
Isidoro Ghezzi
ZmnSCPxj