Browse Source

db: save and restore last_sent_commit correctly.

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>
ppa-0.6.1
Rusty Russell 6 years ago
committed by Christian Decker
parent
commit
cefb6925b2
  1. 2
      Makefile
  2. 1
      tests/test_connection.py
  3. 2
      wallet/db.c
  4. 1
      wallet/test/Makefile
  5. 14
      wallet/test/run-wallet.c
  6. 51
      wallet/wallet.c

2
Makefile

@ -35,7 +35,7 @@ endif
ifeq ($(COMPAT),1)
# We support compatibility with pre-0.6.
COMPAT_CFLAGS=-DCOMPAT_V052=1
COMPAT_CFLAGS=-DCOMPAT_V052=1 -DCOMPAT_V060=1
endif
PYTEST_OPTS := -v -x

1
tests/test_connection.py

@ -1297,7 +1297,6 @@ def test_dataloss_protection(node_factory, bitcoind):
assert (closetxid, "confirmed") in set([(o['txid'], o['status']) for o in l2.rpc.listfunds()['outputs']])
@pytest.mark.xfail(strict=True)
@unittest.skipIf(not DEVELOPER, "needs dev_disconnect")
def test_restart_multi_htlc_rexmit(node_factory, bitcoind, executor):
# l1 disables commit timer once we send first htlc, dies on commit

2
wallet/db.c

@ -337,6 +337,8 @@ char *dbmigrations[] = {
"ALTER TABLE payments ADD description TEXT;",
/* future_per_commitment_point if other side proves we're out of date -- */
"ALTER TABLE channels ADD future_per_commitment_point BLOB;",
/* last_sent_commit array fix */
"ALTER TABLE channels ADD last_sent_commit BLOB;",
NULL,
};

1
wallet/test/Makefile

@ -6,6 +6,7 @@ WALLET_TEST_COMMON_OBJS := \
common/base32.o \
common/derive_basepoints.o \
common/htlc_state.o \
common/htlc_wire.o \
common/type_to_string.o \
common/memleak.o \
common/key_derive.o \

14
wallet/test/run-wallet.c

@ -747,9 +747,10 @@ static bool channelseq(struct channel *c1, struct channel *c2)
CHECK(c1->our_config.id != 0 && c1->our_config.id == c2->our_config.id);
CHECK((lc1 != NULL) == (lc2 != NULL));
if(lc1) {
CHECK(lc1->newstate == lc2->newstate);
CHECK(lc1->id == lc2->id);
CHECK(tal_count(lc1) == tal_count(lc2));
for (size_t i = 0; i < tal_count(lc1); i++) {
CHECK(lc1[i].newstate == lc2[i].newstate);
CHECK(lc1[i].id == lc2[i].id);
}
CHECK((c1->last_tx != NULL) == (c2->last_tx != NULL));
@ -795,7 +796,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx)
struct channel_info *ci = &c1.channel_info;
struct bitcoin_txid *hash = tal(w, struct bitcoin_txid);
struct pubkey pk;
struct changed_htlc last_commit;
struct changed_htlc *last_commit;
secp256k1_ecdsa_signature *sig = tal(w, secp256k1_ecdsa_signature);
u8 *scriptpubkey = tal_arr(ctx, u8, 100);
@ -804,7 +805,8 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx)
memset(ci, 3, sizeof(*ci));
mempat(hash, sizeof(*hash));
mempat(sig, sizeof(*sig));
mempat(&last_commit, sizeof(last_commit));
last_commit = tal_arr(w, struct changed_htlc, 2);
mempat(last_commit, tal_bytelen(last_commit));
pubkey_from_der(tal_hexdata(w, "02a1633cafcc01ebfb6d78e39f687a1f0995c62fc95f51ead10a02ee0be551b5dc", 66), 33, &pk);
ci->feerate_per_kw[LOCAL] = ci->feerate_per_kw[REMOTE] = 31337;
mempat(scriptpubkey, tal_count(scriptpubkey));
@ -864,7 +866,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx)
CHECK(c1.their_shachain.id == 1);
/* Variant 3: update with last_commit_sent */
c1.last_sent_commit = &last_commit;
c1.last_sent_commit = last_commit;
wallet_channel_save(w, &c1);
CHECK_MSG(!wallet_err, tal_fmt(w, "Insert into DB: %s", wallet_err));
CHECK_MSG(c2 = wallet_channel_load(w, c1.dbid), tal_fmt(w, "Load from DB"));

51
wallet/wallet.c

@ -609,13 +609,26 @@ static struct channel *wallet_stmt2channel(const tal_t *ctx, struct wallet *w, s
remote_shutdown_scriptpubkey = sqlite3_column_arr(tmpctx, stmt, 28, u8);
/* Do we have a last_sent_commit, if yes, populate */
if (sqlite3_column_type(stmt, 30) != SQLITE_NULL) {
if (sqlite3_column_type(stmt, 41) != SQLITE_NULL) {
const u8 *cursor = sqlite3_column_blob(stmt, 41);
size_t len = sqlite3_column_bytes(stmt, 41);
size_t n = 0;
last_sent_commit = tal_arr(tmpctx, struct changed_htlc, n);
while (len) {
tal_resize(&last_sent_commit, n+1);
fromwire_changed_htlc(&cursor, &len,
&last_sent_commit[n++]);
}
} else
last_sent_commit = NULL;
#ifdef COMPAT_V060
if (!last_sent_commit && sqlite3_column_type(stmt, 30) != SQLITE_NULL) {
last_sent_commit = tal(tmpctx, struct changed_htlc);
last_sent_commit->newstate = sqlite3_column_int64(stmt, 30);
last_sent_commit->id = sqlite3_column_int64(stmt, 31);
} else {
last_sent_commit = NULL;
}
#endif
if (sqlite3_column_type(stmt, 40) != SQLITE_NULL) {
future_per_commitment_point = tal(tmpctx, struct pubkey);
@ -715,7 +728,8 @@ static const char *channel_fields =
/*30*/ "last_sent_commit_state, last_sent_commit_id, "
/*32*/ "last_tx, last_sig, last_was_revoke, first_blocknum, "
/*36*/ "min_possible_feerate, max_possible_feerate, "
/*38*/ "msatoshi_to_us_min, msatoshi_to_us_max, future_per_commitment_point ";
/*38*/ "msatoshi_to_us_min, msatoshi_to_us_max, future_per_commitment_point, "
/*41*/ "last_sent_commit";
bool wallet_channels_load_active(const tal_t *ctx, struct wallet *w)
{
@ -893,6 +907,7 @@ u64 wallet_get_channel_dbid(struct wallet *wallet)
void wallet_channel_save(struct wallet *w, struct channel *chan)
{
sqlite3_stmt *stmt;
u8 *last_sent_commit;
assert(chan->first_blocknum);
wallet_channel_config_save(w, &chan->our_config);
@ -996,17 +1011,23 @@ void wallet_channel_save(struct wallet *w, struct channel *chan)
db_exec_prepared(w->db, stmt);
/* If we have a last_sent_commit, store it */
if (chan->last_sent_commit) {
stmt = db_prepare(w->db,
"UPDATE channels SET"
" last_sent_commit_state=?,"
" last_sent_commit_id=?"
" WHERE id=?");
sqlite3_bind_int(stmt, 1, chan->last_sent_commit->newstate);
sqlite3_bind_int64(stmt, 2, chan->last_sent_commit->id);
sqlite3_bind_int64(stmt, 3, chan->dbid);
db_exec_prepared(w->db, stmt);
}
last_sent_commit = tal_arr(tmpctx, u8, 0);
for (size_t i = 0; i < tal_count(chan->last_sent_commit); i++)
towire_changed_htlc(&last_sent_commit,
&chan->last_sent_commit[i]);
stmt = db_prepare(w->db,
"UPDATE channels SET"
" last_sent_commit=?"
" WHERE id=?");
if (tal_count(last_sent_commit))
sqlite3_bind_blob(stmt, 1,
last_sent_commit, tal_count(last_sent_commit),
SQLITE_TRANSIENT);
else
sqlite3_bind_null(stmt, 1);
sqlite3_bind_int64(stmt, 2, chan->dbid);
db_exec_prepared(w->db, stmt);
}
void wallet_channel_insert(struct wallet *w, struct channel *chan)

Loading…
Cancel
Save