We say "in N blocks" but we actually mean "N blocks after this tx" which is
actually N-1 or less. Change wording and tighten tests which misunderstood
this.
Also, the 'assert not l1.daemon.is_in_log('onchaind complete, forgetting peer')'
are unlikely to work until the daemon has actually seen the block, so add
sync_blockheight before all of those.
These changes reveal some sloppy testing, which we fix.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
With the following patch applied, we could clearly see onchaind try to
broadcast the timeout tx one block too early:
sendrawtx exit 26, gave error code: -26?error message:?non-final (code 64)?
This is because of an out-by-one error in calculating the relative
depth required, since the out->tx_blockheight is already 1 before the
current block.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was revealed in #1114; onchaind isn't actually completely idempotent
due to fee changes (and the now-fixed change in keys used).
This triggers the bug by restarting with different fees, resulting in
onchaind not recognizing its own proposal:
2018-03-05T09:38:15.550Z lightningd(23076): lightning_onchaind-022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59 chan #1: STATUS_FAIL_INTERNAL_ERROR: THEIR_UNILATERAL/OUR_HTLC spent with weird witness 3
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The root cause of #1114 was that the restarted onchaind created a
different proposal to the one which had previously been mined:
2018-03-01T09:41:08.884Z lightningd(1): lightning_onchaind-020d3d5995a973c878e3f6e5f59da54078304c537f981d7dcef73367ecbea0e90e chan #1: STATUS_FAIL_INTERNAL_ERROR: THEIR_UNILATERAL/OUR_HTLC spent with weird witness 3
After the previous patches which fixed the output address difference,
we could identify proposals by their outputs, but during the
transition (onchaind started with old buggy version, restarted now)
that wouldn't be right, so we match the inputs, discarding signatures
which will be different. This works for all current cases.
Closes: #1114
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It would be better to give them unique values, but we don't fully support
db migrate anyway and this is simple (though they will end up using the
same key for multiple channel closes if created before this commit).
Note that even if bip32_max_index is currently unset, it defaults to 0
so it will be found.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This simplifies things, and means it's always in the database. Our
previous approach to creating it on the fly had holes when it was
created for onchaind, causing us to use another every time we
restarted.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our testing also reveals a bug: we start lightningd and shut it down
before fully processing the blockchain, so we don't set
last_processed_block. Fix that by setting it immediately once we have
a block: worst case it goes backwards a little.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This lets us clearly mark transition features, in a way that they can
be removed after 0.6 is released.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In general, it is true that accessors should take const and discard it,
but chainparams is *always* const.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When we already know about an output we would stop scanning the remaining
outputs. Known outputs happen whenever we extracted from our own transactions
and then extracted again from blocks. We would not update if the first update
fails.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Transaction filters are strongly related to the wallet, this move just
makes it a bit more explicit.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Will be used later to filter out outputs we are interested in, and
trigger db updates with them.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
nicolas.dorier
Christian Decker
Rusty Russell
John Barboza
Douglas Schilling Landgraf
Corné Plooy