diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 835dedcd3..5c21c8b2d 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -287,8 +287,8 @@ static void json_delinvoice(struct command *cmd, } if (!wallet_invoice_remove(cmd->ld->wallet, i)) { - log_broken(cmd->ld->log, "Error attempting to remove invoice %"PRIu64": %s", - i->id, cmd->ld->wallet->db->err); + log_broken(cmd->ld->log, "Error attempting to remove invoice %"PRIu64, + i->id); command_fail(cmd, "Database error"); return; } diff --git a/lightningd/test/run-find_my_path.c b/lightningd/test/run-find_my_path.c index 8700b06d3..001dd9f2d 100644 --- a/lightningd/test/run-find_my_path.c +++ b/lightningd/test/run-find_my_path.c @@ -7,11 +7,11 @@ int unused_main(int argc, char *argv[]); /* Generated stub for crashlog_activate */ void crashlog_activate(const char *argv0 UNNEEDED, struct log *log UNNEEDED) { fprintf(stderr, "crashlog_activate called!\n"); abort(); } -/* Generated stub for db_begin_transaction */ -bool db_begin_transaction(struct db *db UNNEEDED) -{ fprintf(stderr, "db_begin_transaction called!\n"); abort(); } +/* Generated stub for db_begin_transaction_ */ +void db_begin_transaction_(struct db *db UNNEEDED, const char *location UNNEEDED) +{ fprintf(stderr, "db_begin_transaction_ called!\n"); abort(); } /* Generated stub for db_commit_transaction */ -bool db_commit_transaction(struct db *db UNNEEDED) +void db_commit_transaction(struct db *db UNNEEDED) { fprintf(stderr, "db_commit_transaction called!\n"); abort(); } /* Generated stub for debug_poll */ int debug_poll(struct pollfd *fds UNNEEDED, nfds_t nfds UNNEEDED, int timeout UNNEEDED) diff --git a/wallet/db.c b/wallet/db.c index 1d054db6f..86657a1d7 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -122,86 +122,64 @@ char *dbmigrations[] = { NULL, }; -/** - * db_clear_error - Clear any errors from previous queries - */ -static void db_clear_error(struct db *db) -{ - db->err = tal_free(db->err); -} - sqlite3_stmt *db_prepare_(const char *caller, struct db *db, const char *query) { int err; sqlite3_stmt *stmt; - if (db->in_transaction && db->err) - return NULL; - db_clear_error(db); + assert(db->in_transaction); + err = sqlite3_prepare_v2(db->sql, query, -1, &stmt, NULL); - if (err != SQLITE_OK) { - db->err = tal_fmt(db, "%s: %s: %s", caller, query, - sqlite3_errmsg(db->sql)); - } + if (err != SQLITE_OK) + fatal("%s: %s: %s", caller, query, sqlite3_errmsg(db->sql)); + return stmt; } -bool db_exec_prepared_(const char *caller, struct db *db, sqlite3_stmt *stmt) +void db_exec_prepared_(const char *caller, struct db *db, sqlite3_stmt *stmt) { - if (db->in_transaction && db->err) { - goto fail; - } + assert(db->in_transaction); - db_clear_error(db); - - if (sqlite3_step(stmt) != SQLITE_DONE) { - db->err = - tal_fmt(db, "%s: %s", caller, sqlite3_errmsg(db->sql)); - goto fail; - } + if (sqlite3_step(stmt) != SQLITE_DONE) + fatal("%s: %s", caller, sqlite3_errmsg(db->sql)); sqlite3_finalize(stmt); - return true; -fail: - sqlite3_finalize(stmt); - return false; } -bool PRINTF_FMT(3, 4) +/* This one doesn't check if we're in a transaction. */ +static void db_do_exec(const char *caller, struct db *db, const char *cmd) +{ + char *errmsg; + int err; + + err = sqlite3_exec(db->sql, cmd, NULL, NULL, &errmsg); + if (err != SQLITE_OK) { + fatal("%s:%s:%s:%s", caller, sqlite3_errstr(err), cmd, errmsg); + /* Only reached in testing */ + sqlite3_free(errmsg); + } +} + +void PRINTF_FMT(3, 4) db_exec(const char *caller, struct db *db, const char *fmt, ...) { va_list ap; - char *cmd, *errmsg; - int err; + char *cmd; - if (db->in_transaction && db->err) - return false; - - db_clear_error(db); + assert(db->in_transaction); va_start(ap, fmt); cmd = tal_vfmt(db, fmt, ap); va_end(ap); - err = sqlite3_exec(db->sql, cmd, NULL, NULL, &errmsg); - if (err != SQLITE_OK) { - tal_free(db->err); - db->err = tal_fmt(db, "%s:%s:%s:%s", caller, - sqlite3_errstr(err), cmd, errmsg); - sqlite3_free(errmsg); - tal_free(cmd); - return false; - } + db_do_exec(caller, db, cmd); tal_free(cmd); - return true; } bool db_exec_prepared_mayfail_(const char *caller, struct db *db, sqlite3_stmt *stmt) { - if (db->in_transaction && db->err) { - goto fail; - } + assert(db->in_transaction); if (sqlite3_step(stmt) != SQLITE_DONE) { goto fail; @@ -221,10 +199,7 @@ sqlite3_stmt *PRINTF_FMT(3, 4) char *query; sqlite3_stmt *stmt; - if (db->in_transaction && db->err) - return NULL; - - db_clear_error(db); + assert(db->in_transaction); va_start(ap, fmt); query = tal_vfmt(db, fmt, ap); @@ -237,50 +212,20 @@ sqlite3_stmt *PRINTF_FMT(3, 4) static void close_db(struct db *db) { sqlite3_close(db->sql); } -bool db_begin_transaction(struct db *db) +void db_begin_transaction_(struct db *db, const char *location) { - if (!db->in_transaction) { - /* Clear any errors from previous transactions and - * non-transactional queries */ - db_clear_error(db); - db->in_transaction = db_exec(__func__, db, "BEGIN TRANSACTION;"); - assert(db->in_transaction); - return db->in_transaction; - } - db->in_transaction++; - return false; + if (db->in_transaction) + fatal("Already in transaction from %s", db->in_transaction); + + db_do_exec(location, db, "BEGIN TRANSACTION;"); + db->in_transaction = location; } -bool db_commit_transaction(struct db *db) -{ - bool ret; - - assert(db->in_transaction); - if (db->err) { - char *errmsg; - int err; - - /* Do this manually: db_exec is a NOOP with db->err */ - err = sqlite3_exec(db->sql, "ROLLBACK;", NULL, NULL, &errmsg); - if (err != SQLITE_OK) { - db->err = tal_fmt(db, "%s then ROLLBACK failed:%s:%s", - db->err, sqlite3_errstr(err), errmsg); - sqlite3_free(errmsg); - } - ret = false; - } else { - ret = db_exec(__func__, db, "COMMIT;"); - } - db->in_transaction--; - return ret; -} - -bool db_rollback_transaction(struct db *db) +void db_commit_transaction(struct db *db) { assert(db->in_transaction); - bool ret = db_exec(__func__, db, "ROLLBACK;"); - db->in_transaction--; - return ret; + db_exec(__func__, db, "COMMIT;"); + db->in_transaction = NULL; } /** @@ -308,11 +253,8 @@ static struct db *db_open(const tal_t *ctx, char *filename) db->filename = tal_dup_arr(db, char, filename, strlen(filename), 0); db->sql = sql; tal_add_destructor(db, close_db); - db->in_transaction = false; - db->err = NULL; - if (!db_exec(__func__, db, "PRAGMA foreign_keys = ON;")) { - fatal("Could not enable foreignkeys on database: %s", db->err); - } + db->in_transaction = NULL; + db_do_exec(__func__, db, "PRAGMA foreign_keys = ON;"); return db; } @@ -411,16 +353,14 @@ s64 db_get_intvar(struct db *db, char *varname, s64 defval) return res; } -bool db_set_intvar(struct db *db, char *varname, s64 val) +void db_set_intvar(struct db *db, char *varname, s64 val) { /* Attempt to update */ db_exec(__func__, db, "UPDATE vars SET val='%" PRId64 "' WHERE name='%s';", val, varname); - if (sqlite3_changes(db->sql) > 0) - return true; - else - return db_exec( + if (sqlite3_changes(db->sql) == 0) + db_exec( __func__, db, "INSERT INTO vars (name, val) VALUES ('%s', '%" PRId64 "');", diff --git a/wallet/db.h b/wallet/db.h index a5c047c98..8b8e3f89f 100644 --- a/wallet/db.h +++ b/wallet/db.h @@ -15,8 +15,7 @@ struct db { char *filename; - unsigned int in_transaction; - const char *err; + const char *in_transaction; sqlite3 *sql; }; @@ -25,6 +24,7 @@ struct db { * * Opens the database, creating it if necessary, and applying * migrations until the schema is updated to the current state. + * Calls fatal() on error. * * Params: * @ctx: the tal_t context to allocate from @@ -33,37 +33,33 @@ struct db { struct db *db_setup(const tal_t *ctx); /** - * db_query - Prepare and execute a query, and return the result + * db_query - Prepare and execute a query, and return the result (or NULL) */ sqlite3_stmt *PRINTF_FMT(3, 4) db_query(const char *caller, struct db *db, const char *fmt, ...); -bool PRINTF_FMT(3, 4) +/** + * db_exec - execute a statement, call fatal() if it fails. + */ +void PRINTF_FMT(3, 4) db_exec(const char *caller, struct db *db, const char *fmt, ...); /** * db_begin_transaction - Begin a transaction * - * Begin a new DB transaction if we aren't already in one. Returns - * true if the call started a transaction, i.e., the caller MUST take - * care to either commit or rollback. If false, this is a nested - * transaction and the caller MUST not commit/rollback, since the - * transaction is handled at a higher level in the callstack. + * Begin a new DB transaction. fatal() on database error. */ -bool db_begin_transaction(struct db *db); +#define db_begin_transaction(db) \ + db_begin_transaction_((db), __FILE__ ":" stringify(__LINE__)) +void db_begin_transaction_(struct db *db, const char *location); /** * db_commit_transaction - Commit a running transaction * - * Requires that we are currently in a transaction. Returns whether - * the commit was successful. + * Requires that we are currently in a transaction. fatal() if we + * fail to commit. */ -bool db_commit_transaction(struct db *db); - -/** - * db_rollback_transaction - Whoops... undo! undo! - */ -bool db_rollback_transaction(struct db *db); +void db_commit_transaction(struct db *db); /** * db_set_intvar - Set an integer variable in the database @@ -71,7 +67,7 @@ bool db_rollback_transaction(struct db *db); * Utility function to store generic integer values in the * database. */ -bool db_set_intvar(struct db *db, char *varname, s64 val); +void db_set_intvar(struct db *db, char *varname, s64 val); /** * db_get_intvar - Retrieve an integer variable from the database @@ -102,18 +98,18 @@ sqlite3_stmt *db_prepare_(const char *caller, struct db *db, const char *query); * After preparing a statement using `db_prepare`, and after binding * all non-null variables using the `sqlite3_bind_*` functions, it can * be executed with this function. It is a small, transaction-aware, - * wrapper around `sqlite3_step`, that also sets `db->err` if the - * execution fails. This will take ownership of `stmt` and will free + * wrapper around `sqlite3_step`, that calls fatal() if the execution + * fails. This will take ownership of `stmt` and will free * it before returning. * * @db: The database to execute on * @stmt: The prepared statement to execute */ #define db_exec_prepared(db,stmt) db_exec_prepared_(__func__,db,stmt) -bool db_exec_prepared_(const char *caller, struct db *db, sqlite3_stmt *stmt); +void db_exec_prepared_(const char *caller, struct db *db, sqlite3_stmt *stmt); /** - * db_exec_prepared_mayfail - db_exec_prepared, but don't set db->err if it fails. + * db_exec_prepared_mayfail - db_exec_prepared, but don't fatal() it fails. */ #define db_exec_prepared_mayfail(db,stmt) \ db_exec_prepared_mayfail_(__func__,db,stmt) diff --git a/wallet/db_tests.c b/wallet/db_tests.c index 83feee23c..2df08caf9 100644 --- a/wallet/db_tests.c +++ b/wallet/db_tests.c @@ -1,3 +1,8 @@ + #include + +static void db_fatal(const char *fmt, ...); +#define fatal db_fatal + #include "db.c" #include "wallet/test_utils.h" @@ -5,6 +10,19 @@ #include #include +static char *db_err; +static void db_fatal(const char *fmt, ...) +{ + va_list ap; + + /* Fail hard if we're complaining about not being in transaction */ + assert(!strstarts(fmt, "No longer in transaction")); + + va_start(ap, fmt); + db_err = tal_vfmt(NULL, fmt, ap); + va_end(ap); +} + static struct db *create_test_db(const char *testname) { struct db *db; @@ -23,9 +41,13 @@ static bool test_empty_db_migrate(void) { struct db *db = create_test_db(__func__); CHECK(db); + db_begin_transaction(db); CHECK(db_get_version(db) == -1); + db_commit_transaction(db); db_migrate(db); + db_begin_transaction(db); CHECK(db_get_version(db) == db_migration_count()); + db_commit_transaction(db); tal_free(db); return true; @@ -34,17 +56,22 @@ static bool test_empty_db_migrate(void) static bool test_primitives(void) { struct db *db = create_test_db(__func__); - CHECK_MSG(db_begin_transaction(db), "Starting a new transaction"); + db_begin_transaction(db); CHECK(db->in_transaction); - CHECK_MSG(db_commit_transaction(db), "Committing a transaction"); + db_commit_transaction(db); CHECK(!db->in_transaction); - CHECK_MSG(db_begin_transaction(db), "Starting a transaction after commit"); - CHECK(db_rollback_transaction(db)); + db_begin_transaction(db); + db_commit_transaction(db); - CHECK_MSG(db_exec(__func__, db, "SELECT name FROM sqlite_master WHERE type='table';"), "Simple correct SQL command"); - CHECK_MSG(!db_exec(__func__, db, "not a valid SQL statement"), "Failing SQL command"); + db_begin_transaction(db); + db_exec(__func__, db, "SELECT name FROM sqlite_master WHERE type='table';"); + CHECK_MSG(!db_err, "Simple correct SQL command"); + + db_exec(__func__, db, "not a valid SQL statement"); + CHECK_MSG(db_err, "Failing SQL command"); + db_err = tal_free(db_err); + db_commit_transaction(db); CHECK(!db->in_transaction); - CHECK_MSG(db_begin_transaction(db), "Starting a transaction after a failed transaction"); tal_free(db); return true; @@ -57,16 +84,18 @@ static bool test_vars(void) CHECK(db); db_migrate(db); + db_begin_transaction(db); /* Check default behavior */ CHECK(db_get_intvar(db, varname, 42) == 42); /* Check setting and getting */ - CHECK(db_set_intvar(db, varname, 1)); + db_set_intvar(db, varname, 1); CHECK(db_get_intvar(db, varname, 42) == 1); /* Check updating */ - CHECK(db_set_intvar(db, varname, 2)); + db_set_intvar(db, varname, 2); CHECK(db_get_intvar(db, varname, 42) == 2); + db_commit_transaction(db); tal_free(db); return true; diff --git a/wallet/wallet.c b/wallet/wallet.c index e80d942c2..bbce351d4 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -106,7 +106,7 @@ static void unreserve_utxo(struct wallet *w, const struct utxo *unres) if (!wallet_update_output_status(w, &unres->txid, unres->outnum, output_state_reserved, output_state_available)) { - fatal("Unable to unreserve output: %s", w->db->err); + fatal("Unable to unreserve output"); } } @@ -126,7 +126,7 @@ void wallet_confirm_utxos(struct wallet *w, const struct utxo **utxos) if (!wallet_update_output_status( w, &utxos[i]->txid, utxos[i]->outnum, output_state_reserved, output_state_spent)) { - fatal("Unable to mark output as spent: %s", w->db->err); + fatal("Unable to mark output as spent"); } } } @@ -154,7 +154,7 @@ const struct utxo **wallet_select_coins(const tal_t *ctx, struct wallet *w, if (!wallet_update_output_status( w, &available[i]->txid, available[i]->outnum, output_state_available, output_state_reserved)) - fatal("Unable to reserve output: %s", w->db->err); + fatal("Unable to reserve output"); weight += (32 + 4 + 4) * 4; if (utxos[i]->is_p2sh) @@ -229,19 +229,16 @@ s64 wallet_get_newindex(struct lightningd *ld) return newidx; } -bool wallet_shachain_init(struct wallet *wallet, struct wallet_shachain *chain) +void wallet_shachain_init(struct wallet *wallet, struct wallet_shachain *chain) { sqlite3_stmt *stmt; /* Create shachain */ shachain_init(&chain->chain); stmt = db_prepare(wallet->db, "INSERT INTO shachains (min_index, num_valid) VALUES (?, 0);"); sqlite3_bind_int64(stmt, 1, chain->chain.min_index); - if (!db_exec_prepared(wallet->db, stmt)) { - return false; - } + db_exec_prepared(wallet->db, stmt); chain->id = sqlite3_last_insert_rowid(wallet->db->sql); - return true; } /* TODO(cdecker) Stolen from shachain, move to some appropriate location */ @@ -971,7 +968,7 @@ bool wallet_htlcs_load_for_channel(struct wallet *wallet, DIRECTION_INCOMING, chan->id, SENT_REMOVE_ACK_REVOCATION); if (!stmt) { - log_broken(wallet->log, "Could not select htlc_ins: %s", wallet->db->err); + log_broken(wallet->log, "Could not select htlc_ins"); return false; } @@ -992,7 +989,7 @@ bool wallet_htlcs_load_for_channel(struct wallet *wallet, DIRECTION_OUTGOING, chan->id, RCVD_REMOVE_ACK_REVOCATION); if (!stmt) { - log_broken(wallet->log, "Could not select htlc_outs: %s", wallet->db->err); + log_broken(wallet->log, "Could not select htlc_outs"); return false; } @@ -1058,8 +1055,6 @@ void wallet_invoice_save(struct wallet *wallet, struct invoice *inv) if (!inv->id) { stmt = db_prepare(wallet->db, "INSERT INTO invoices (payment_hash, payment_key, state, msatoshi, label) VALUES (?, ?, ?, ?, ?);"); - if (!stmt) - fatal("Could not prepare statement: %s", wallet->db->err); sqlite3_bind_blob(stmt, 1, &inv->rhash, sizeof(inv->rhash), SQLITE_TRANSIENT); sqlite3_bind_blob(stmt, 2, &inv->r, sizeof(inv->r), SQLITE_TRANSIENT); @@ -1067,21 +1062,16 @@ void wallet_invoice_save(struct wallet *wallet, struct invoice *inv) sqlite3_bind_int64(stmt, 4, inv->msatoshi); sqlite3_bind_text(stmt, 5, inv->label, strlen(inv->label), SQLITE_TRANSIENT); - if (!db_exec_prepared(wallet->db, stmt)) - fatal("Could not exec prepared statement: %s", wallet->db->err); + db_exec_prepared(wallet->db, stmt); inv->id = sqlite3_last_insert_rowid(wallet->db->sql); } else { stmt = db_prepare(wallet->db, "UPDATE invoices SET state=? WHERE id=?;"); - if (!stmt) - fatal("Could not prepare statement: %s", wallet->db->err); - sqlite3_bind_int(stmt, 1, inv->state); sqlite3_bind_int64(stmt, 2, inv->id); - if (!db_exec_prepared(wallet->db, stmt)) - fatal("Could not exec prepared statement: %s", wallet->db->err); + db_exec_prepared(wallet->db, stmt); } } @@ -1109,7 +1099,7 @@ bool wallet_invoices_load(struct wallet *wallet, struct invoices *invs) "SELECT id, state, payment_key, payment_hash, " "label, msatoshi FROM invoices;"); if (!stmt) { - log_broken(wallet->log, "Could not load invoices: %s", wallet->db->err); + log_broken(wallet->log, "Could not load invoices"); return false; } @@ -1131,7 +1121,8 @@ bool wallet_invoice_remove(struct wallet *wallet, struct invoice *inv) { sqlite3_stmt *stmt = db_prepare(wallet->db, "DELETE FROM invoices WHERE id=?"); sqlite3_bind_int64(stmt, 1, inv->id); - return db_exec_prepared(wallet->db, stmt) && sqlite3_changes(wallet->db->sql) == 1; + db_exec_prepared(wallet->db, stmt); + return sqlite3_changes(wallet->db->sql) == 1; } struct htlc_stub *wallet_htlc_stubs(tal_t *ctx, struct wallet *wallet, @@ -1143,9 +1134,6 @@ struct htlc_stub *wallet_htlc_stubs(tal_t *ctx, struct wallet *wallet, "SELECT channel_id, direction, cltv_expiry, payment_hash " "FROM channel_htlcs WHERE channel_id = ?;"); - if (!stmt) - fatal("Error preparing select: %s", wallet->db->err); - sqlite3_bind_int64(stmt, 1, chan->id); stubs = tal_arr(ctx, struct htlc_stub, 0); diff --git a/wallet/wallet.h b/wallet/wallet.h index 65924d07d..6573f345f 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -141,7 +141,7 @@ s64 wallet_get_newindex(struct lightningd *ld); /** * wallet_shachain_init -- wallet wrapper around shachain_init */ -bool wallet_shachain_init(struct wallet *wallet, struct wallet_shachain *chain); +void wallet_shachain_init(struct wallet *wallet, struct wallet_shachain *chain); /** * wallet_shachain_add_hash -- wallet wrapper around shachain_add_hash diff --git a/wallet/wallet_tests.c b/wallet/wallet_tests.c index b88d6b555..2165a8cf5 100644 --- a/wallet/wallet_tests.c +++ b/wallet/wallet_tests.c @@ -22,13 +22,16 @@ static void wallet_fatal(const char *fmt, ...) /* Fail hard if we're complaining about not being in transaction */ assert(!strstarts(fmt, "No longer in transaction")); + /* Fail hard if we're complaining about not being in transaction */ + assert(!strstarts(fmt, "No longer in transaction")); + va_start(ap, fmt); wallet_err = tal_vfmt(NULL, fmt, ap); va_end(ap); } #define transaction_wrap(db, ...) \ - (db_begin_transaction(db), __VA_ARGS__, db_commit_transaction(db)) + (db_begin_transaction(db), __VA_ARGS__, db_commit_transaction(db), wallet_err == NULL) void invoice_add(struct invoices *invs, struct invoice *inv){} @@ -85,6 +88,8 @@ static bool test_wallet_outputs(void) memset(&u, 0, sizeof(u)); + db_begin_transaction(w->db); + /* Should work, it's the first time we add it */ CHECK_MSG(wallet_add_utxo(w, &u, p2sh_wpkh), "wallet_add_utxo failed on first add"); @@ -117,6 +122,7 @@ static bool test_wallet_outputs(void) output_state_spent), "could not change output state ignoring oldstate"); + db_commit_transaction(w->db); tal_free(w); return true; } @@ -143,7 +149,10 @@ static bool test_shachain_crud(void) memset(&b, 0, sizeof(b)); w->db = db_open(w, filename); - CHECK(wallet_shachain_init(w, &a)); + db_begin_transaction(w->db); + CHECK_MSG(!wallet_err, "db_begin_transaction failed"); + wallet_shachain_init(w, &a); + CHECK(!wallet_err); CHECK(a.id == 1); @@ -158,6 +167,9 @@ static bool test_shachain_crud(void) CHECK(wallet_shachain_load(w, a.id, &b)); CHECK_MSG(memcmp(&a, &b, sizeof(a)) == 0, "Loading from database doesn't match"); + + db_commit_transaction(w->db); + CHECK(!wallet_err); tal_free(w); return true; } @@ -261,11 +273,16 @@ static bool test_channel_crud(const tal_t *ctx) ci.remote_per_commit = pk; ci.old_remote_per_commit = pk; + db_begin_transaction(w->db); + CHECK(!wallet_err); + /* Variant 1: insert with null for scid, funding_tx_id, channel_info, last_tx */ wallet_channel_save(w, &c1); CHECK_MSG(!wallet_err, - tal_fmt(w, "Insert into DB: %s", w->db->err)); - CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB: %s", w->db->err)); + tal_fmt(w, "Insert into DB: %s", wallet_err)); + CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB")); + CHECK_MSG(!wallet_err, + tal_fmt(w, "Load from DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v1)"); /* We just inserted them into an empty DB so this must be 1 */ @@ -277,8 +294,10 @@ static bool test_channel_crud(const tal_t *ctx) c1.peer->scid = talz(w, struct short_channel_id); wallet_channel_save(w, &c1); CHECK_MSG(!wallet_err, - tal_fmt(w, "Insert into DB: %s", w->db->err)); - CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB: %s", w->db->err)); + tal_fmt(w, "Insert into DB: %s", wallet_err)); + CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB")); + CHECK_MSG(!wallet_err, + tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v2)"); /* Updates should not result in new ids */ @@ -290,39 +309,51 @@ static bool test_channel_crud(const tal_t *ctx) c1.peer->our_msatoshi = &msat; wallet_channel_save(w, &c1); - CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", w->db->err)); - CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB: %s", w->db->err)); + CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); + CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB")); + CHECK_MSG(!wallet_err, + tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v3)"); /* Variant 4: update with funding_tx_id */ c1.peer->funding_txid = hash; wallet_channel_save(w, &c1); - CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", w->db->err)); - CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB: %s", w->db->err)); + CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); + CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB")); + CHECK_MSG(!wallet_err, + tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v4)"); /* Variant 5: update with channel_info */ p.channel_info = &ci; wallet_channel_save(w, &c1); - CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", w->db->err)); - CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB: %s", w->db->err)); + CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); + CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB")); + CHECK_MSG(!wallet_err, + tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v5)"); /* Variant 6: update with last_commit_sent */ p.last_sent_commit = &last_commit; wallet_channel_save(w, &c1); - CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", w->db->err)); - CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB: %s", w->db->err)); + CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); + CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB")); + CHECK_MSG(!wallet_err, + tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v6)"); /* Variant 7: update with last_tx (taken from BOLT #3) */ p.last_tx = bitcoin_tx_from_hex(w, "02000000000101bef67e4e2fb9ddeeb3461973cd4c62abb35050b1add772995b820b584a488489000000000038b02b8003a00f0000000000002200208c48d15160397c9731df9bc3b236656efb6665fbfe92b4a6878e88a499f741c4c0c62d0000000000160014ccf1af2f2aabee14bb40fa3851ab2301de843110ae8f6a00000000002200204adb4e2f00643db396dd120d4e7dc17625f5f2c11a40d857accc862d6b7dd80e040047304402206a2679efa3c7aaffd2a447fd0df7aba8792858b589750f6a1203f9259173198a022008d52a0e77a99ab533c36206cb15ad7aeb2aa72b93d4b571e728cb5ec2f6fe260147304402206d6cb93969d39177a09d5d45b583f34966195b77c7e585cf47ac5cce0c90cefb022031d71ae4e33a4e80df7f981d696fbdee517337806a3c7138b7491e2cbb077a0e01475221023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb21030e9f7b623d2ccc7c9bd44d66d5ce21ce504c0acf6385a132cec6d3c39fa711c152ae3e195220", strlen("02000000000101bef67e4e2fb9ddeeb3461973cd4c62abb35050b1add772995b820b584a488489000000000038b02b8003a00f0000000000002200208c48d15160397c9731df9bc3b236656efb6665fbfe92b4a6878e88a499f741c4c0c62d0000000000160014ccf1af2f2aabee14bb40fa3851ab2301de843110ae8f6a00000000002200204adb4e2f00643db396dd120d4e7dc17625f5f2c11a40d857accc862d6b7dd80e040047304402206a2679efa3c7aaffd2a447fd0df7aba8792858b589750f6a1203f9259173198a022008d52a0e77a99ab533c36206cb15ad7aeb2aa72b93d4b571e728cb5ec2f6fe260147304402206d6cb93969d39177a09d5d45b583f34966195b77c7e585cf47ac5cce0c90cefb022031d71ae4e33a4e80df7f981d696fbdee517337806a3c7138b7491e2cbb077a0e01475221023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb21030e9f7b623d2ccc7c9bd44d66d5ce21ce504c0acf6385a132cec6d3c39fa711c152ae3e195220")); p.last_sig = sig; wallet_channel_save(w, &c1); - CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", w->db->err)); - CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB: %s", w->db->err)); + CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err)); + CHECK_MSG(wallet_channel_load(w, c1.id, c2), tal_fmt(w, "Load from DB")); + CHECK_MSG(!wallet_err, + tal_fmt(w, "Insert into DB: %s", wallet_err)); CHECK_MSG(channelseq(&c1, c2), "Compare loaded with saved (v7)"); + db_commit_transaction(w->db); + CHECK(!wallet_err); tal_free(w); return true; } @@ -346,7 +377,7 @@ static bool test_channel_config_crud(const tal_t *ctx) cc1->id == 1, tal_fmt(ctx, "channel_config->id != 1; got %" PRIu64, cc1->id)); - CHECK(wallet_channel_config_load(w, cc1->id, cc2)); + CHECK(transaction_wrap(w->db, wallet_channel_config_load(w, cc1->id, cc2))); CHECK(memeq(cc1, sizeof(*cc1), cc2, sizeof(*cc2))); return true; } @@ -363,7 +394,8 @@ static bool test_htlc_crud(const tal_t *ctx) struct htlc_out_map *htlcs_out = tal(ctx, struct htlc_out_map); /* Make sure we have our references correct */ - db_exec(__func__, w->db, "INSERT INTO channels (id) VALUES (1);"); + CHECK(transaction_wrap(w->db, + db_exec(__func__, w->db, "INSERT INTO channels (id) VALUES (1);"))); chan->id = 1; chan->peer = peer; @@ -383,11 +415,13 @@ static bool test_htlc_crud(const tal_t *ctx) /* Store the htlc_in */ CHECK_MSG(transaction_wrap(w->db, wallet_htlc_save_in(w, chan, &in)), - tal_fmt(ctx, "Save htlc_in failed: %s", w->db->err)); + tal_fmt(ctx, "Save htlc_in failed: %s", wallet_err)); CHECK_MSG(in.dbid != 0, "HTLC DB ID was not set."); /* Saving again should get us a collision */ CHECK_MSG(!transaction_wrap(w->db, wallet_htlc_save_in(w, chan, &in)), "Saving two HTLCs with the same data must not succeed."); + CHECK(wallet_err); + wallet_err = tal_free(wallet_err); /* Update */ CHECK_MSG(transaction_wrap(w->db, wallet_htlc_update(w, in.dbid, RCVD_ADD_HTLC, NULL)), @@ -397,21 +431,28 @@ static bool test_htlc_crud(const tal_t *ctx) "Update HTLC with payment_key failed"); CHECK_MSG(transaction_wrap(w->db, wallet_htlc_save_out(w, chan, &out)), - tal_fmt(ctx, "Save htlc_out failed: %s", w->db->err)); + tal_fmt(ctx, "Save htlc_out failed: %s", wallet_err)); CHECK_MSG(out.dbid != 0, "HTLC DB ID was not set."); CHECK_MSG(!transaction_wrap(w->db, wallet_htlc_save_out(w, chan, &out)), "Saving two HTLCs with the same data must not succeed."); + CHECK(wallet_err); + wallet_err = tal_free(wallet_err); /* Attempt to load them from the DB again */ htlc_in_map_init(htlcs_in); htlc_out_map_init(htlcs_out); + db_begin_transaction(w->db); + CHECK(!wallet_err); + CHECK_MSG(wallet_htlcs_load_for_channel(w, chan, htlcs_in, htlcs_out), "Failed loading HTLCs"); CHECK_MSG(wallet_htlcs_reconnect(w, htlcs_in, htlcs_out), "Unable to reconnect htlcs."); + db_commit_transaction(w->db); + CHECK(!wallet_err); hin = htlc_in_map_get(htlcs_in, &in.key); hout = htlc_out_map_get(htlcs_out, &out.key);