We weren't incrementing the `col` for the `local_shutdown_idx` field,
which meant that all following fields were incorrect. I removed the
`col` computation and opted for absolute indices instead, since they
are way less brittle. Just remember to add new fields to the query at
the end so we don't have to shift too often :-)
Reported-by: William Casarin @jb55
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This addresses a performance regression introduced by
6ceb375650. We were storing it in an
otherwise empty DB transaction, which means that DB transaction was no
longer a no-op. Now we defer storing until we need to store the
corresponding HTLC anyway, so we can just piggyback on top of that
transaction.
This is also more consistent since we'd be forgetting the payment
anyway if we restart between adding the HTLC and committing to it.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This seems to happen when we manage to check between the
channel_announcement and the channel_update being processed.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Add two simple tests: one for a single direct payment and one with
hundreds of parallel payments, reusing the same route.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We only send them when we're not awaiting revoke_and_ack: our
simplified handling can't deal with multiple in flights.
Closes: #244
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can have it happen on reconnect due to fee changes; we should really
detect this case, but it's harmless to let it happen as a noop.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Handling feerates for the fundee (who only receives fee_update) is
simple: it's practically atomic since we accept commitment and send
revocation, thus they're applied to both sides at once.
Handling feerates for the funder is more complex: in theory we could
have multiple in flight. However, if we avoid this using the same
logic as we use to suppress multiple commitments in flight, it's
simple again.
We fix the test code to use real feerate manipulation, thus have to
remove an assert about feerate being non-zero. And now we have
feechanges, we need to rely on the changes_pending flags, as we can
have changes without an HTLCs changing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The wire protocol uses this, in the assumption that we'll never see feerates
in excess of 4294967 satoshi per kiloweight.
So let's use that consistently internally as well.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Depending on what we're doing, we can want different ones. So use
IMMEDIATE (estimatesmartfee 2 CONSERVATIVE), NORMAL (estimatesmartfee
4 ECONOMICAL) and SLOW (estimatesmartfee 100 ECONOMICAL).
If one isn't available, we try making each one half the previous.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means we convert it when retrieving from bitcoind; internally it's
always satoshi-per-1000-weight aka millisatoshi-per-weight.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Test objects must be added to $(ALL_OBJS) so they correctly depend on
CCAN headers etc.
Also, each test in a subdir must depend on headers and src in the parent
directory, as it will often #include them directly.
Reported-by: Christian Decker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our testsuite uses --dev-fail-on-subdaemon-fail, so I didn't notice this
until I turned that off to chase a bug.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We currently scan through HTLCs: this isn't enough if we've only got a
feechange in the commitment, so use a flag (but keep both for now for
debugging).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Used by the JSON-RPC for the listtransfers call. Currently does not
support any form of paging.
Signed-off-by: Christian Decker <decker.christian@gmail.com>