This increments the `data_version` upon committing dirty transactions, reads
the last data_version upon startup, and tracks the number in memory in
parallel to the DB (see next commit for rationale).
Changelog-Changed: JSON-RPC: Added a `data_version` field to the `db_write` hook which returns a numeric transaction counter.
The upgrade here is a bit tricky: we map the two values into the
feerate_state. This is trivial if they're both the same, but if
they're different we don't know exactly what state they're in (this
being the source of the bug!).
So, we assume that the have received the update and not acked it,
as that would be the normal case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is in preparation for partial payments. For existing payments,
partid is 0 (to match the corresponding payment).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is in preparation for partial payments. For existing payments,
partid is 0 (arbitrarity) and total_msat is msatoshi.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In a future version, we will use features to insist that payers
provide the secret. In transition, we may have old invoices which
didn't insist on that, so we need to know this on a per-invoice basis.
Not sure if I got the right syntax for adding an empty blob though!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Printed form is always "[<nodeid>-]<prefix>: <string>"
2. "jcon fd %i" becomes "jsonrpc #%i".
3. "jsonrpc" log is only used once, and is removed.
4. "database" log prefix is use for db accesses.
5. "lightningd(%i)" becomes simply "lightningd" without the pid.
6. The "lightningd_" prefix is stripped from subd log prefixes, and pid removed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-changed: Logging: formatting made uniform: [NODEID-]SUBSYSTEM: MESSAGE
Changelog-removed: `lightning_` prefixes removed from subdaemon names, including in listpeers `owner` field.
The case where this is needed is when the wallet had a forwarded payment
somewhere between commits 66a47d2 (which started tracking forwardings) and
d901304 (which added the `received_time` column). This just emulates the
behavior of sqlite3 for postgres as well.
Signed-off-by: Christian Decker <@cdecker>
Checking on whether we access a null field is ok, but should we crash right
away? Probably not. This reduces the access to a warning on sqlite3 and let's
it continue. We can look for occurences and fix them as they come up and then
re-arm the asserts once we addressed all cases.
We were implicitly relying on sqlite3 behavior that returns the zero-value for
nulled fields when accessing them. This adds the same behavior explicitly to
the DB abstraction in order to reduce `db_column_is_null` checks in the logic,
but still make it evident what is happening here.
Fixes https://github.com/fiatjaf/mcldsp/issues/1
Signed-off-by: Christian Decker <@cdecker>
`shutdown_scriptpubkey[REMOTE]` is original remote_shutdown_scriptpubkey;
`shutdown_scriptpubkey[LOCAL]` is the script used for "to-local" output when `close`. Add the default is generated form `final_key_idx`;
Store `shutdown_scriptpubkey[LOCAL]` into wallet;
sqlite3 was forgiving, postgres isn't, so let's make sure we use the strictest
field type possible, relaxing when rewriting.
The commit consists just of the following mapping
- INTEGER -> BIGSERIAL if it is the primary key
- INTEGER -> BIGINT if it is an amount or a reference to a primary key
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This was already done in `db_step` but `db_count_changes` and
`db_last_insert_id` also rely on the statement being executed. Furthermore we
now check that the statement was executed before freeing it, so it can't
happen that we dispose of a statement we meant to execute but forgot.
The combination of these could be used to replace the pending_statement
tracking based on lists, since we now make sure to execute all statements and
we use the memleak checker to make sure we don't keep a statement in memory.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
sqlite3 doesn't really do any validation whatsoever, and there is no
difference between 64bit and 32bit numbers. Posgtres on the other hand gets
very upset if the size doesn't match.
This commit swaps out handwavy types with the ones that should be there :-)
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This was weird right from the start, so we just split the table into integers
and blobs, so each column has a well-defined format. It is also required for
postgres not to cry about explicit casts in the `paramTypes` array.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
The first ever query to check if the version DB exists may fail. We allow
this, but we need to restart the DB transaction since postgres fails the
current transaction and rolls back any changes.
This just commits (and fails) and starts a new transaction so the rest of the
migration can continue.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Needed to change a couple of migrations. The changes are mostly innocuous:
- changing BLOB to TEXT for short_channel_ids which is the correct type
anyway, and sqlite3 treats them the same anyway.
- Use `int` for version since the byte representation is checked by postgres.
- Change anything that is INT, but will be bound to u64 to BIGINT (again
postgres checks these more carefully than sqlite3).
Two migrations were replaced with dummy values, since they are buried deep
enough, and I found no portable way of expressing `strftime()` and `INSERT OR
IGNORE`.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Using a generated identifier with filename and line proved to be brittle since
compilers assign the __LINE__ macro differently on multi-line macro
invocations.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is dangerous but needed since postgres is not as forgiving about
unsatisfied foreign key constraints even while in a DB transaction.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We used to do some of the setup work in db.c, which is now free of any
sqlite3-specific code. In addition we also switch over to fully qualified DSNs
to specify the location of the wallet.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We're about to remove them.
Includes fix to sqlite3_bind_short_channel_id to not assume `id` is a
tal object.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now that all the users are migrated to the abstraction layer we can remove the
legacy implementation.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
It's better to let the driver decide when and how to expand. It can then
report the expanded statement back to the dispatch through the
`db_changes_add` function.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We now have a much stronger consistency check from the combination of
transaction wrapping, tal memory leak detection. Tramsaction wrapping ensures
that each statement is executed before the transaction is committed. The
commit is also driven by the `io_loop`, which means that it is no longer
possible for us to have statements outside of transactions and transactions
are guaranteed to commit at the round's end.
By adding the tal-awareness we can also get a much better indication as to
whether we have un-freed statements flying around, which we can test at the
end of the round as well.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is likely the last part we need to completely encapsulate the part of the
sqlite3 API that we were using. Like the `db_count_changes` call I decided to
pass in the `struct db_stmt` since really they refer to the statement that was
executed and not the db.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
These are based on top of the basic column access functions, and act as a
small type-safe wrapper, that also does a bit of validation.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This has a slight side-effect of removing the actual begin and commit
statements from the `db_write` hooks, but they are mostly redundant anyway (no
harm in grouping pre-init statements into one transaction, and we know that
each post-init call is supposed to be wrapped anyway).
Signed-off-by: Christian Decker <decker.christian@gmail.com>
These are used to do one-time initializations and wait for pending statements
before closing.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
I was hoping to get rid of these by using "ON CONFLICT" upserts, however
sqlite3 only started supporting them in version 3.24.0 which is newer than
some of our deployment targets.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is the first step towards being able to extract information from query
rows. Only the most basic types are exposed, the others will be built on top
of these primitives.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
For some of the query methods in the next step we need to have an idea of
whether the stmt was executed (db_step function) so let's track that
explicitly.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
These do not require the ability to iterate over the result, hence they can be
migrated already.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
These functions implement the lookup of the query, and the dispatch to the
DB-specific functions that do the actual heavy lifting.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
All drivers will have to reach into it, so put it in a place that is reachable
from the drivers, along with all other definitions.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is the counterpart of the annotations we did in the last few commits. It
extracts queries, passes them through a driver-specific query rewriter and
dumps them into a driver-specific query-list, along with some metadata to
facilitate processing later on. The generated query list is then registered as
a `db_config` and will be loaded by the driver upon instantiation.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We will soon generalize the DB, so directly reaching into the `struct db`
instance to talk to the sqlite3 connection is bad anyway. This increases
flexibility and allows us to tailor the actual implementation to the
underlying DB.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
`db_select_prepare` was prepending the "SELECT" part in an attempt to limit
its use to read-only statements. This is leads to the queries in the code not
actually being well-formed, which we'll need in a later commit, and was also
resulting in extra allocations. This switches the behavior to just enforce a
"SELECT" prefix being present which allows us to have well-formed queries in
the code again and avoids the extra allocation.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We need to have full DB queries that can be extracted at compile time later in
order to be able to rewrite them in other SQL dialects. In addition we had a
bit of unnecessary code-duplication in db_select and db_select_prepare. Now
the former uses the latter internally.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Dumb programs which have a --daemon option call fork() early. This is
terrible UX since startup errors get lost: the program exits with
"success" immediately then you discover via the logs that it didn't
start at all.
However, forking late introduced a heap of problems with changing
pids. Instead, fork early but keep stderr and the parent around: if
we fail early on, the parent fails with us. We release our parent
with an explicit action just before the main loop.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are generalized from our internal implementations.
The main difference is that 'struct json_escaped' is now 'struct
json_escape', so we replace that immediately.
The difference between lightningd's json-writing ringbuffer and the
more generic ccan/json_out is that the latter has a better API and
handles escaping transparently if something slips through (though
it does offer direct accessors so you can mess things up yourself!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Mainly used to differentiate channel-related transactions from on-chain wallet
transactions. Will be used to filter `listtransaction` results and bundle
transactions that belong to the same channel.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We'd like to display the receive and resolution times in the forwardings
table. In order to remember the receive time we need to store it in the DB
along with the other information.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Node ids are pubkeys, but we only use them as pubkeys for routing and checking
gossip messages. So we're packing and unpacking them constantly, and wasting
some space and time.
This introduces a new type, explicitly the SEC1 compressed encoding
(33 bytes). We ensure its validity when we load from the db, or get it
from JSON. We still use 'struct pubkey' for peer messages, which checks
validity.
Results from 5 runs, min-max(mean +/- stddev):
store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec
39475-39572(39518+/-36),2880732,41.150000-41.390000(41.298+/-0.085),2.260000-2.550000(2.336+/-0.11),44.390000-65.150000(58.648+/-7.5),32.740000-33.020000(32.89+/-0.093),44.130000-45.090000(44.566+/-0.32)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Pubkeys are not not actually DER encoding, but Pieter Wuille corrected
me: it's SEC 1 documented encoding.
Results from 5 runs, min-max(mean +/- stddev):
store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec
38922-39297(39180.6+/-1.3e+02),2880728,41.040000-41.160000(41.106+/-0.05),2.270000-2.530000(2.338+/-0.097),44.570000-53.980000(49.696+/-3),32.840000-33.080000(32.95+/-0.095),43.060000-44.950000(43.696+/-0.72)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
wallet/test/run-wallet was failing the valgrind check; turns out
`sqlite3_expanded_sql` expects you to manage the memory of strings
it returns. from `sqlite3.h`:
** ^The string returned by sqlite3_expanded_sql(P), on the other hand,
** is obtained from [sqlite3_malloc()] and must be free by the application
** by passing it to [sqlite3_free()].
I was tempted to create a new db_select_stmt wrapper type, but that means
a lot of boilerplate around binding, which expects to work with db_prepare
*and* db_select_prepare.
This lets us clearly differentiate between db queries (which don't need to
go to a plugin) and db changes (which do).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
- Intrduce DB update `channel` values: `feerate_base` and `feerate_ppm`
- Make fist use of now context realted DB migration
- Add `struct channel` members of the same name
- Use struct values instead of config when commiting new channels
Allow a function as well as (or instead of!) an sql statement. That
will let us do things like set per-channel values to the global
defaults, for example.
Since we remove the NULL termination, the final entry is ARRAY_SIZE()-1
not ARRAY_SIZE()-2.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
More efficient to measure the ARRAY_SIZE(), which is a runtime
constant. We move it into the unit test.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Below this code appears:
if (current != orig)
db_exec(__func__, db,
"INSERT INTO db_upgrades VALUES (%i, '%s');",
orig, version());
But since the loop pre-increments current, this is always true. I wondered
why there were so many duplicates in my db_upgrades table!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This field was used by `pay` to hold the bolt11 description if the bolt11
string used `h` to hash the description (which nobody ever did). If the
`h` field wasn't present, it could contain anything, as it wasn't checked.
It's really useful to have a label for payments (eg. '1 Cuban'), but adding
yet-another option would be painful, so we simply rename 'description'
to 'label' except inside the db.
This means we need to do some tricky parameter parsing to handle array
and keyword JSON arguments, but only until we remove the old name.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Without this, there's no proof of payment, since it is the signed invoice
that make the receipt valid.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In order to avoid having to ask the HSM for public keys to
their_unilateral/to_us outputs we just store the `scriptPubkey` with the UTXO,
which can then be converted to the P2WPKH address.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We need to do it in various places, but we shouldn't do it lightly:
the primitives are there to help us get overflow handling correct.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were not correctly allocating the `db->filename`, failing to copy the
null-terminator. This was causing and error when reopening the database after
the call to `fork()`.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Reported-by: Sean McNally <@sfmcnally>
Changelog-fixed: Fixed a crash when running in daemon-mode due to db filename overrun
We need to still accept it when parsing the database, but this flag
should allow upgrade testing for devs building on top
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian and I both unwittingly used it in form:
*tal_arr_expand(&x) = tal(x, ...)
Since '=' isn't a sequence point, the compiler can (and does!) cache
the value of x, handing it to tal *after* tal_arr_expand() moves it
due to tal_resize().
The new version is somewhat less convenient to use, but doesn't have
this problem, since the assignment is always evaluated after the
resize.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
json_escaped.[ch], param.[ch] and jsonrpc_errors.h move from lightningd/
to common/. Tests moved too.
We add a new 'common/json_tok.[ch]' for the common parameter parsing
routines which a plugin might want, taking them out of
lightningd/json.c (which now only contains the lightningd-specific
ones).
The rest is mainly fixing up includes.
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
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's an array: we were only saving the single element; if there was more than
one changed HTLC we'd get a bad signature!
The report in #1907 is probably caused by the other side re-requesting
something we considered already finalized; to avoid this particular error,
we should set the field to NULL if there's no last_sent_commit.
I'm increasingly of the opinion we want to just save all the update
packets to the db and blast them out, instead of doing this
second-guessing dance.
Fixes: #1907
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means we don't try to unilaterally close after a restart, *and*
we can tell onchaind to try to use the point to recover funds when the
peer unilaterally closes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're currently overriding fatal() with something that actually
returns, which contrasts with its declaration as NORETURN.
This breaks in the next patch which wants a real fatal() in wallet.h.
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>
