This fixes half of #59 (the part which crashes). It doesn't fix the
fact that we should never be doing this for testnet.
Reported-by: Thomas Daede <daede003@umn.edu>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Allocate the route off the current command, not dstate. And in the
case where the route is somehow not via a peer, don't leak memory.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Generally, the pattern is: everything returned is allocated off the return
value, which is the only thing allocated off the context. And it's always
freed.
Also, tal_free() returns NULL, so it's useful for one-line error
cleanups.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This shouldn't happen, but if our RNG is busted and get a duplicate,
r would be NULL.
(scan-build found this)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`listinvoice` was not handling numeric labels correctly if they are not
passed in as JSON strings (which was the behavior of
`lightning-cli`). So now we accept both string labels as well as numeric
labels.
The JSON-RPC was non-standard compliant in that it omitted the required
`jsonrpc` entry and it was returning both `error` as well as
`result`. This fixes both of these issues.
Fixes#50. Was causing a segfault because it was creating an empty route
and was trying to extract the first hop as next hop. Routes through self
can still be created manually, but `sendpay` would still refuse to act
on them due to the missing self-link.
We're going to wean off IRC, but as a quick fix, only announce 0-60 seconds
after we see a join, or every 6 hours.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We've seen intermittant failures on testnet, so disable sending feechanges
for now: we're completely changing it for 0.6 anyway, due to Milan Spec.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make this always the first packet after auth. That means there's no
reliance on whether a node remembers an aborted connection.
It also gives us a place to put version bits.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This gives much better errors, and allows us to return the peer id.
Closes: #37
Reported-by: Glenn Willen
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
My lightning node was *way* behind, and I couldn't figure out why it
wasn't seeing channel establishment.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, remove fee ranges on testnet (too unreliable) and accept
a single confirm.
(Note that an earlier version of this had a bug when there was no
config file, this version includes the fix).
Closes: #40
Reported-by: Glenn Willen
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Firstly, we need to update the staging fee amount when we queue a change.
Secondly we need to remove completed fee updates, otherwise we hit a
database constraint that peer & state are unique.
Reported-by: Christian Decker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were out by 1000, and also derived it from the previous, not current
state.
Reported-by: Christian Decker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We store peers in the database for STATE_INIT, but they don't reconnect
properly. We should not forget STATE_INIT dropped peers, but use some
timeout mechanism if we can't reconnect to clean up.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Three days of on and off debugging, before I realized my server was talking
to a non-testnet bitcoind. There was a bitcoind on that machine running
on testnet, but it uses the same dir and config, so the --bitcoin-datadir
option couldn't help.
This is more certain: specify whether we're testnet on every single query.
Now we can skip the attempt to parse bitcoin.conf, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
getnodes returns an object containing a single array of 'nodes'. Each
element contains the node's ID, its hostname and its port. If
unknown (because we haven't seen a node announcement yet) then the port
is 0 and the hostname is null.
lightningd now uses a WHOIS query on itself to learn its external IP
address and announces that on the channel with the NODE message. It also
tracks other nodes in the routing table.
Refactored the signature verification to reuse it for both CHAN and NODE
messages.
'getchannels' returns a 'channels' array containing an object for each
known channel. Each channel object represents one direction of a
bidirectional channel, with a from and a to node ID along with the fees
for that direction. This matched the internal storage of channels and
allows unbalanced fees for each direction.
We *should* be in a state which accepts it (could happen with reorg),
and there's no reason to test for greater than depth since we must process
blocks in order.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Currently we get the odd message "Own anchor has insufficient funds".
Reported-by: Christian Decker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
NO_VALGRIND= daemon/test/test.sh --normal --restart
lightning-cli: Connecting to 'lightning-rpc': Connection refused
lightning-cli: Connecting to 'lightning-rpc': Connection refused
lightning-cli: Connecting to 'lightning-rpc': Connection refused
lightning-cli: Connecting to 'lightning-rpc': Connection refused
lightning-cli: Connecting to 'lightning-rpc': Connection refused
This is expected: it happens when node3 is restarting. Redirect
errors to /dev/null.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Added channel announcement serialization and parsing, as well as the
entrypoints for the IRC peer discovery. Announcements are signed by the
sending endpoint and signatures are verified before adding the channels
to the local view of the topology. We do not yet verify the existence of
the anchor transaction.
It's not in a transaction in one caller, so wrap that.
This removes some more error handling code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rename the structs to match (and remove dev-echo).
This makes it clear that they're not the normal API.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need some ordering to deliver them to the JSON "waitinvoice" command;
we use a counter where 0 means "unpaid".
We keep two lists now, one for unpaid and one for paid invoices.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need some way to reflect the tradeoff between the possible delay if
a payment gets stuck, and the fees charged by nodes. This adds a risk
factor which reflects the probability that a node goes down, and the
cost associated with losing access to our funds for a given time.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I had some nonsensical columns, eg "bool ours", but sqlite3 pretty much
ignores them. Use macros so mistakes are harder to make.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is important when we put payments in the database: they need to be
updated atomically as the HTLC is.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is important when we put payments in the database: they need to be
updated atomically as the HTLC is.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had enum channel_side (OURS, THEIRS) for which end of a channel we
had, and htlc_side (LOCAL, REMOTE) for who proposed the HTLC.
Combine these both into simply "enum side".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, make sure B can just afford it, then have the A add a
HTLC which means B can no longer afford the fees, and A should cover
it.
We do this by modifying the previous overlapping-fail test, but we
need to have B offer it the htlc before A does: racy in the normal
autocommit case. So we do a manual commit here, always.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When they propose an HTLC to us, they need to be able to cover both it,
and the associated fees. When it gets acked and applied to them, however,
they may no longer be able to afford the fees; this is OK and expected.
So add a flag to say whether they can dig into fees or not: without
this patch the code calls fatal() on the next patch which tests it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We create a logging object when we connect, then carry it through. If
it comes from the database, we just use the peerid as the log prefix.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we haven't received their closing signature yet, we might try to
send the closing packet anyway (and segfault). Make sure we have
their signature before trying that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is less convenient to use, but makes far more sense for a real
user (like a wallet). It can ask about the route, then decide whether
to use it or not.
This will make even more sense once we add a parameter to control how
long we let the HTLC be delayed for, so a client can query for high,
medium and low tolerances and compare results.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We stopped automatically retransmitting locally-generated add/removes
after a reconnect, but this breaks the "pay" interface as it stands.
The correct solution to this is to make the pay interface idempotent:
you can trigger it as many times as you want and it will only succeed
once.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we've not relayed a failure yet (ie. we relayed it instantly, but it
wasn't confirmed), we need to redo it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's not currently encrypted, but at least you get some idea now why
an HTLC failed. We (ab)use HTTP error codes for the moment.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These low level commands we restarted on reconnect for ease of
testing. Don't do that, and check that we're connected when those
commands occur.
This introduces subtle issues with --manual-commit --reconnect: restarting
node1 also forgets uncommitted things from node2, requiring reordering for
some tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We capture the output in case we need to resubmit the command after restarting,
but we weren't printing it out on failure (set -e means we'd stop immediately).
As a side-effect of this change, we don't restart after failed
commands, which caused another bug: we were writing the 2->3 route to
the config file, but not restarting again, so we lost the route.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we send an HTLC #1, then get disconnected before a confirm, we will
forget it. But we've incremented peer->htlc_id_counter, so when we offer
it again we'll make it HTLC #2, which is non-consecutive.
To make this clear, we always start htlc ids at 0 now. That revealed
the bugs handled in the previous patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't have an ordering of HTLCs between peers: we don't know
whether your HTLC 0 or my HTLC 0 occurred first. This matters,
as we play them all back to reconstruct state (probably overkill anyway).
So we add force_* operators which don't do bounds checks, and do
bounds checks at the end. We also note that we don't need to apply
fee changes: that should be in the database already.
Also relax db constraints: IDs are not unique, they are unique per
side (we can both have HTLC #0).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The first hop is being stripped from computed routes, however the
first channel of the route is being used to get our peer address. This
results in segfaults if the route is just one hop, i.e., has no first
channel to get the peer's address from. Fixed by simply using an
existing pointer to our peer.
I originally overloaded struct htlc for this, as they go through the
same states, but separating them turned out to be clearer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows hardcoded routes in the config file, which is required until
we get route advertisements.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes more sense eventually: we may know the network addresses of
many peers, not just those we're connecting to. So keep a mapping, and
update it when we successfully connect outwards.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Testing this revealed that we can't just reconnect when we have something to
send, as we might be NATed; we should try to reconnect anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We add a "dev-restart" command which causes the daemon to close fds
and exec itself. Then we do it after every command, with the caveat
that we always send a commit before newhtlc, because if not committed,
that is forgotten. Fulfillhtlc and failhtlc get resent, since they're
idempotent.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Fix #ifdef DEBUG code in signature.c so it compiles.
2. Don't set peer->closing.our_script in queue_pkt_close_shutdown: it's
assigned in caller already.
3. Wrap setting of htlc's rval in set_htlc_rval() function.
4. Log where we were when unexpected packet comes in.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
More of a pure allocator, for when we load peers from db. Also moves
shachain_init out of secrets and into new_peer where it logically
belongs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
So if there are no HTLCs, and the receiver can't spend anyway, don't
sign. This has the added benefit that no two signed commitment
transactions will ever be identical (the revocation preimage changes).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This gives us a clear way to indicate "invalid", and also sqlite3 stores
signed 64-bit numbers, so it's clearer this way.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It fits in a u32, but we mix it with other values which could cause
overflow, so let's just use u64 everywhere.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is dumb, since one side will never succeed. But in future when
there is a method for nodes to broadcast their public address (or send
their address inline to connected nodes), either side should try to
connect.
Importantly though, there are places which will queue packets at
various times (eg. HTLC timeout), so we need to clear the queue just
before re-transmitting, not when disconnecting.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
To do this we keep an order counter so we know how to retransmit. We
could simply keep old packets, but this is a little clearer for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Caught because we generated an HTLCs which had already expired, since
we didn't know the latest block. Other errors are certainly possible,
so it's safest to load the entire thing before going live.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids us having to query it when we create anchor transaction, and
lets us always use dynamic fee information.
The config options for max and min are now percentages, rather than absolute.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We no longer need it anywhere. This simplifies things to the point where
we might as well just not include dust outputs as we go, rather than
explicitly removing them, which gets rid of remove_dust.c as well.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Similar to the way we derive which outputs are which for old transactions
we steal, we derive them even for their current transaction.
We keep track of this information in peer->closing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
At the moment, for our or their unilateral close, we create a resolved[]
entry for our output, their output, and each HTLC, in cstate order. Some
of these outputs might not exist (too small), so it's actually better
to simply keep a resolved[] entry for each of the tx's actual outputs.
(We already changed the steal resolved[] array to work like this, but
these are trickier, since we rely on that order if we need to fulfill an
on-chain HTLC).
It also helps as we are weaning off knowing the cstate and permutation
mapping for each commitment transaction.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to stop keeping old commitment information (except the minimal
txid to commitment-number mapping). One place we currently use it is
after sending a commitment signature, and before we've received the
revocation for the old commitment. For this duration, there are two
valid commitment transactions.
So we store "their_prev_revocation_hash" explicitly for this duration.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's a data-leak to send ack before we have verified identity of peer.
Plus, we can't send it until we know which peer it is, anyway!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And use this to resolve old transactions by comparing outputs with
HTLCs.
Rather than remembering the output ordering for every one of their
previous commitment transactions, we just remember the commitment
number for each commitment txid, and when we see it, derive all the
HTLC scriptpubkeys and the to-us and to-them scriptpubkeys, and figure
out which is which.
This avoids us having to save information on disk, except for the
txid->commitment-number mapping (and the shachain).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes it explicit, which is better for storing in a database (before
it was just what watch callback, plus peer->local.mindepth).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Move other logic into caller, but it's not complete (it still needs to
check some things, and still records some results).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Move other logic into caller: it grew this way because we used to have
a centralized "state" machine which knew nothing of these internal
details. But now we want to re-queue packets on reconnect, we really
want these routines to be idempotent.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're weaning off the cstate arrays; use the htlc map. But for the
moment we keep the output basically the same.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had an occasional race where we hadn't gotten the remote revocation
before submitting fulfill (spotted by the HTLC state transition code).
Disallow this, but also add to the json output so we can wait for
an HTLC to be irrevocably committed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we always remove " from JSON, our parsing becomes simpler; turns
out that we did that in some places, and check()'s eval removed them
from the comparison.
We extract check_balance_single() to check the general balance, then
grep for HTLCs separately.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Not separate "locally-offered" and "remotely-offered" ones; we can
distinguish them by htlc->state now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since we only care about the latest commits, we can simply associate a
state with each HTLC, rather than using queues of HTLCs associated
with each commitment transaction.
This works far better in the context of a database.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need SO_REUSEADDR, and we need to memset sockaddr to zero; valgrind
complains for both IPv4 and IPv6, but the invalid sin6_flowinfo causes
the IPv6 bind to fail altogether.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
getchaintips returns tips even if we don't have the body for them, so
we need to look for the active tip, not just the first (most-work) one.
Here's what happens in the log:
+2849.963414597 lightningd(26779):BROKEN: bitcoin-cli getblock 0000000000000000018626ff7160bdf38a602e6650bd04ec258759ea578b106d false exited 91: 'error code: -32603
error message:
Can't read block from disk
'
And here's an example problematic getchaintips output:
[
{
"height": 419635,
"hash": "0000000000000000000fd32d87fce19efb7ccd07aa4ddaf1b94b9a219deec0f9",
"branchlen": 1,
"status": "headers-only"
},
{
"height": 419634,
"hash": "000000000000000002988d6512719697147cf252b2f64d247cf229266615d2bb",
"branchlen": 0,
"status": "active"
},
{
"height": 416372,
"hash": "0000000000000000004d0a54341c992ae174a91c8dd3981a9f2b3d3f6221ba59",
"branchlen": 1,
"status": "valid-headers"
},
{
"height": 416231,
"hash": "0000000000000000044d0d2c25f33cb48931540366149cde3fb0154f55b58c76",
"branchlen": 1,
"status": "headers-only"
}
]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
From doing a code walkthrough with Christian Decker; unnecessary const in
bitcoin/tx.c, an erroneous FIXME, a missing comment, and an unused struct.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is generally redundant, since HTLC pointer is in that side's
commit_info, but makes HTLC completely self-contained.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Update libsecp256k1 has a normalize function, which allows us to test
if the signature was in low-S form.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Add Makefile target update-secp256k1, and run it.
The only API change is that len is now an IN-OUT parameter to serialization
functions.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We use libsecp256k1 to convert signatures to DER; we were creating a
temporary one, but we really should be handing the one we have in dstate
through. This does that, everywhere.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
BOLT has been updated, so update code and comments. The receiving
side check is sufficient, as the limit is per-offerer, and that's the
only way the HTLCs get back to the offerer's side.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Thus a node MUST estimate the deadline for successful redemption for
each HTLC it offers. A node MUST NOT offer a HTLC after this
deadline, and MUST fail the connection if an HTLC which it offered is
in either node's current commitment transaction past this deadline.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The most complex thing we've logged yet, so we extract the core of the
log_struct_ function to call it multiple times.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There's a corner case where they had it in their commit tx, in which
case we can't fail the HTLC until our commit tx has won. Again, we
use dstate->config.min_htlc_expiry.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We now need to use bitcoin_witness_htlc with the r value, so that API
is updated to take 'struct rval' or 'struct sha256'.
We use the nc->delay amount (ie. dstate->config.min_htlc_expiry) to
wait for a timeout refund to be buried before "failing" upstream.
This should probably be made into a clearer parameter rather than
overloading this one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Header from folded patch 'dont-use-peer-nc-in-onchain-code.patch':
peer: Don't use peer->nc->delay for onchain case.
Use the config var directly. We should be freeing peer->nc when the
connection dies anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the command an actual user would use: it figures out the fee
and route, and pays it if it can.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If a block triggers two peers to close, we ran io_break() on both of them; the
second overrode the first and we didn't end up freeing that one.
Rather than chase such bugs in future, simply iterate to see if any
peers need freeing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that the base fee is in millisatoshi, the proportional fee is
in microsatoshi per satoshi. ie. 1,000,000 means charge 1 satoshi for
every satoshi carried.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Most HTLCs we offer are triggered by an incoming HTLC from a different
peer. Save this "source" htlc, so we can fail/fulfill it when we
fail/fulfill this one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
No more copies!
I tried changing the cstate->side[].htlcs to htlc_map rather than a
simple pointer array, but we rely on those array indices heavily for
permutation mapping, and it turned into a major rewrite (especially
for the steal case).
Eventually, we're going to want to reconstruct the commit info for
older commit txs rather than keeping all the permutation and
per-commit-info HTLC information in memory, so we can do the work
then.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We currently keep copies of HTLCs in each commit_info structure, but
that's redundant. Keep per-peer per-direction maps of HTLCs, then we can
just throw pointers around (next patch).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's a more logical name, and a more logical place. We change
"funding" to "channel" in the remaining exposed symbols, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're about to change the code so that if it can't route, it will fail
the HTLC. The current low-level tests will hate this, so have a dev switch
to turn that off.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the logical place for it to belong: with the HTLC. For the manually-created
HTLCs, we create a simple one-hop route.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
