It's only used for tests, but it's better to use the wallet_channels_load_active like
the real code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's just a sha256_double, but importantly when we convert it to a
string (in type_to_string, which is used in logging) we use
bitcoin_txid_to_hex() so it's reversed as people expect.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I prefer the typesafety of specific functions, rather than having the
caller know that txids are traditionally reversed in bitcoin.
And we already have a bitcoin_txid_to_hex() function for this.
Closes: #411
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It looks like we were missing the address on insert into the peers table. This
will insert the address formatted by fmt_wireaddr. This happens to include the
ip and port.
This is fine, since parse_wireaddr has been updated to parse ports in ip address
strings.
Signed-off-by: William Casarin <jb55@jb55.com>
* Add port parsing support to parse_wireaddr. This is in preparation for storing
addresses in the peers table. This also makes parse_wireaddr a proper inverse of
fmt_wireaddr.
* Move parse_wireaddr to common/wireaddr.c this seems like a better place for
it. I bring along parse_ip_port with it for convenience. This also fixes some
issues with the upcoming ip/port parsing tests.
Signed-off-by: William Casarin <jb55@jb55.com>
Accuracy improvements:
1. We assumed the output was a p2wpkh, but it can be user-supplied now.
2. We assumed we always had change; remove this for wallet_select_all.
Calculation out-by-one fixes:
1. We need to add 1 byte (4 sipa) for the input count.
2. We need to add 1 byte (4 sipa) for the output count.
3. We need to add 1 byte (4 sipa) for the output script length for each output.
4. We need to add 1 byte (4 sipa) for the input script length for each input.
5. We need to add 1 byte (4 sipa) for the PUSH optcode for each P2SH input.
The results are now a slight overestimate (due to guessing 73 bytes
for signature, whereas they're 71 or 72 in practice).
Fixes: #458
Reported-by: Jonas Nick @jonasnick
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were using changekey as the flag to produce change, not changesat,
but the caller was using changesat as the flag.
Also, don't allocate changekey at all if we don't need it; this means
valgrind will complain if we use it at all, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Two changes:
- Fixed the function signature of noleak_ to match in both
configurations
- Added memleak.o to linker for tests
Generating the stubs for the unit tests doesn't really work since the
stubs are checked in an differ between the two configurations, so
adding memleak to the linker fixes that, by not requiring stubs to be
generated in the first place.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is a primitive mark-and-sweep-style garbage detector. The core is
in common/ for later use by subdaemons, but for now it's just lightningd.
We initialize it before most other allocations.
We walk the tal tree to get all the pointers, then search the `ld`
object for those pointers, recursing down. Some specific helpers are
required for hashtables (which stash bits in the unused pointer bits,
so won't be found).
There's `notleak()` for annotating things that aren't leaks: things
like globals and timers, and other semi-transients.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
jsonrpc handlers usually directly call command_success or
command_fail; not doing that implies they're waiting for something
async.
Put an explicit call (currently a noop) there, and add debugging
checks to make sure it's used.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
bitcoin_from_base58 returns a testnet flag, but json_withdraw
did not actually check it. Add a basic check that the given
withdraw address belongs to the same net lightningd is using.
Change all calls to use the correct serialization and deserialization
functions, include the correct headers and remove the control
messages.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
The filter is being populated while initializing the daemon and by
adding new keys as they are being generated. The filter is then used
in connect_block to identify transactions of interest.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
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>
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>
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>
We save location where transaction was started, in case we try to nest.
There's now no error case; db_exec_mayfail() is the only one.
This means the tests need to override fatal() if they want to intercept
these errors.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to be always in a transaction soon.
Note the rollback we used to do was an optimization: the utxo destructors
would already clean up the new UTXOs in the database.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
we should never be doing two startups at once, but why take chances? Plus,
we can then assert that all db calls are in transactions.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the only case where we actually rely on the db to ensure we don't
do something twice: don't error out if it fails.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Otherwise we find ourselves outside a commitment. This is a bandaid
until we remove nested commitments again at the end of this series.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Nesting is provided by only actually performing the outermost
transaction and simulating the nested ones. This still allows us to
ensure on lower levels that we are in the context of a transaction
without having to resort to keeping explicitly track of it in the
calling code.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
In addition we also set some of the test values to a pattern instead
of just `memset`ting it to 0, which may hide some crossed lines.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We use these quite often and it is cumbersome having to do these
simple conversions inline, so just expose pseudo-sqlite3 methods to
bind and extract from/to a stmt.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Technically it's the caller that'll own the statement, but it is nice
to have db_exec_prepared dispose of it.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
"near \"AND\": syntax error"
This was caught by the "always keep errors for db_commit_transaction".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We'd like to not keep them in memory and retrieve them on-demand when
`onchaind` is launched. This uses the `channel_htlcs` table as backing
but only fetches the minimal necessary information.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This makes executing a query/command a two step process, but allows us
to use the native binding and avoid having to build queries as SQL
strings. Two major advantages are that we are no longer vulnerable to
SQL injections and that we do not have to hex-encode binary fields
like private keys, hashes, and routing onions, halving the storage
requirements for those.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This was causing me some trouble by making it look like the last query
failed, when it really was an old one. No need to drag failures around
for longer than needed.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is a necessary evil since at the time we load `struct htlc_out`
associated with a channel we might not have loaded the `struct
htlc_in` that it depends on, so we defer the rewiring until we have
loaded all HTLCs for all channels. At that point rewiring MUST work,
otherwise we report a failure.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
While loading HTLCs from the database we might not yet have all the
incoming HTLCs loaded when loading a dependent htlc_out. So we defer
the wiring of the HTLCs until we are sure we have them loaded.
This is also the first step towards keeping that association only in
the database, since otherwise we cannot selectively load channels from
DB.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Also added a small warning to one of the used enums not to reorder or
insert values. They'd break the update path.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Addresses #207 by adding a method to retrieve available funds from the
wallet.
Reported-by: @jl777
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This was causing some compilation trouble on 32bit systems, see #256.
Reported-by: @shsmith
Signed-off-by: Christian Decker <decker.christian@gmail.com>