Skip to content

Store onchaind state in the database and reduce rescan on startup #1398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 25, 2018
Merged

Store onchaind state in the database and reduce rescan on startup #1398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
fb8af75
onchaind: Move preimage transfer into onchaind startup
cdecker Apr 2, 2018
3b3f62f
db: Add table for onchaind transactions
cdecker Apr 3, 2018
961e55b
wallet: Add a struct to represent an onchaind transaction
cdecker Apr 3, 2018
2bdbcc4
wallet: Add primitives to store onchaind transactions in the DB
cdecker Apr 3, 2018
57f9129
onchaind: Add a level of indirection to txwatches and txowatches
cdecker Apr 16, 2018
e147967
channel: Allow channel lookup by database id
cdecker Apr 5, 2018
84376f8
pytest: test onchaind restarts from the DB
cdecker Apr 11, 2018
ea97404
onchaind: Store channeltxs so we can restore later
cdecker Apr 17, 2018
155335e
onchaind: Replay stored channeltxs to restore onchaind state
cdecker Apr 17, 2018
01cb08a
wallet: Report current blockheight as the offset to continue from
cdecker Apr 17, 2018
17b3286
opts: Add the --rescan option
cdecker Apr 18, 2018
47c4e74
chaintopology: Simplify rescan offset computation
cdecker Apr 20, 2018
8ef2e8a
pytest: Add a test for the new --rescan option
cdecker Apr 20, 2018
ddd53f1
pytest: Start nodes with --rescan=1
cdecker Apr 21, 2018
fc2b6c0
db: Added DB migrations to get the correct sync height
cdecker Apr 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions wallet/wallet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <lightningd/log.h>
#include <lightningd/peer_control.h>
#include <lightningd/peer_htlcs.h>
#include <onchaind/gen_onchain_wire.h>
#include <string.h>

#define SQLITE_MAX_UINT 0x7FFFFFFFFFFFFFFF
Expand Down Expand Up @@ -2277,3 +2278,77 @@ struct bitcoin_txid *wallet_transactions_by_height(const tal_t *ctx,
return txids;
}

void wallet_channeltxs_add(struct wallet *w, struct channel *chan,
const int type, const struct bitcoin_txid *txid,
const u32 input_num, const u32 blockheight)
{
sqlite3_stmt *stmt;
stmt = db_prepare(w->db, "INSERT INTO channeltxs ("
" channel_id"
", type"
", transaction_id"
", input_num"
", blockheight"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent comma usage

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I started using your comma placement, which is much nicer, half way through and forgot to fixup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another wrinkle of comma placement, is that one notices that SQL consistently, uses longer command keywords, than the keywords for clauses: SELECT vs WHERE vs ORDER vs FROM; INSERT vs VALUES; UPDATE vs WHERE vs SET, etc.

SELECT col1
     , col2
  FROM table
 WHERE col3 = 'whatever'
   AND col4 = 42
     ;

This has the advantage of never requiring spaces before the end of strings being concatenated, since everything after the initial command keyword gets an extra space before it:

"SELECT col1"
"     , col2"
"  FROM table"
" WHERE col3 = 'whatever'"
"   AND col4 = 42"
"     ;"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a bit much imho, with the spacing that I added we actually get the same spacing as if we'd written it in the shell (.schema will show excessive spaces in the CREATE statement for example, which is hard to read).

") VALUES (?, ?, ?, ?, ?);");
sqlite3_bind_int(stmt, 1, chan->dbid);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the 'id' database field used at all?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an autoincrement field that is used in the SELECT below to make sure we replay the events in the same order they were created, i.e., seen originally. This is just for added consistency should we have some dependency on the order (multiple transactions being confirmed in the same block).

Copy link
Contributor

@ZmnSCPxj ZmnSCPxj Apr 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is declared INTEGER PRIMARY KEY, but note, this implies it is equivalent to ROWID (an internal detail) under SQLITE. In particular, if a row is deleted, it is possible for SQLITE to reuse that row when another row is inserted. We need an explicit AUTOINCREMENT in the declaration of id in CREATE TABLE channeltxs.

https://www.sqlite.org/autoinc.html

If the largest ROWID is equal to the largest possible integer (9223372036854775807) then the database engine starts picking positive candidate ROWIDs at random until it finds one that is not previously used. If no unused ROWID can be found after a reasonable number of attempts, the insert operation fails with an SQLITE_FULL error.

...skip...

The normal ROWID selection algorithm described above will generate monotonically increasing unique ROWIDs as long as you never use the maximum ROWID value and you never delete the entry in the table with the largest ROWID. If you ever delete rows or if you ever create a row with the maximum possible ROWID, then ROWIDs from previously deleted rows might be reused when creating new rows and newly created ROWIDs might not be in strictly ascending order.

Admittedly the condition of reaching 2^63 is extremely unlikely, and if we neglect that, we can generally think of INTEGER PRIMARY KEY as implicitly AUTOINCREMENT, but I think explicitness is a win here.

Indeed, later on it pretty much admits that the only difference between INTEGER PRIMARY KEY and INTEGER PRIMARY KEY AUTOINCREMENT is that the latter immediately fails hard when 2^63 is reached, whereas the former will try to recover at the cost of violating monotonically-increasing numbers.

sqlite3_bind_int(stmt, 2, type);
sqlite3_bind_sha256(stmt, 3, &txid->shad.sha);
sqlite3_bind_int(stmt, 4, input_num);
sqlite3_bind_int(stmt, 5, blockheight);

db_exec_prepared(w->db, stmt);
}

u32 *wallet_onchaind_channels(struct wallet *w,
const tal_t *ctx)
{
sqlite3_stmt *stmt;
size_t count = 0;
u32 *channel_ids = tal_arr(ctx, u32, 0);
stmt = db_prepare(w->db, "SELECT DISTINCT(channel_id) FROM channeltxs WHERE type = ?;");
sqlite3_bind_int(stmt, 1, WIRE_ONCHAIN_INIT);

while (sqlite3_step(stmt) == SQLITE_ROW) {
count++;
tal_resize(&channel_ids, count);
channel_ids[count-1] = sqlite3_column_int64(stmt, 0);
}

return channel_ids;
}

struct channeltx *wallet_channeltxs_get(struct wallet *w, const tal_t *ctx,
u32 channel_id)
{
sqlite3_stmt *stmt;
size_t count = 0;
struct channeltx *res = tal_arr(ctx, struct channeltx, 0);
stmt = db_prepare(w->db,
"SELECT"
" c.type"
", c.blockheight"
", t.rawtx"
", c.input_num"
", c.blockheight - t.blockheight + 1 AS depth"
", t.id as txid "
"FROM channeltxs c "
"JOIN transactions t ON t.id == c.transaction_id "
"WHERE channel_id = ? "
"ORDER BY c.id ASC;");
sqlite3_bind_int(stmt, 1, channel_id);

while (sqlite3_step(stmt) == SQLITE_ROW) {
count++;
tal_resize(&res, count);

res[count-1].channel_id = channel_id;
res[count-1].type = sqlite3_column_int(stmt, 0);
res[count-1].blockheight = sqlite3_column_int(stmt, 1);
res[count-1].tx = sqlite3_column_tx(ctx, stmt, 2);
res[count-1].input_num = sqlite3_column_int(stmt, 3);
res[count-1].depth = sqlite3_column_int(stmt, 4);
sqlite3_column_sha256(stmt, 5, &res[count-1].txid.shad.sha);
}

return res;
}
21 changes: 20 additions & 1 deletion wallet/wallet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ struct channel_stats {
u64 out_msatoshi_offered, out_msatoshi_fulfilled;
};

struct onchaindtx {
struct channeltx {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this particular change can be put in the previous commit instead?

u32 channel_id;
int type;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int type always makes me wince. I prefer an enum, if only for readability.

u32 blockheight;
Expand Down Expand Up @@ -846,4 +846,23 @@ struct bitcoin_txid *wallet_transactions_by_height(const tal_t *ctx,
struct wallet *w,
const u32 blockheight);

/**
* Store transactions of interest in the database to replay on restart
*/
void wallet_channeltxs_add(struct wallet *w, struct channel *chan,
const int type, const struct bitcoin_txid *txid,
const u32 input_num, const u32 blockheight);

/**
* List channels for which we had an onchaind running
*/
u32 *wallet_onchaind_channels(struct wallet *w,
const tal_t *ctx);

/**
* Get transactions that we'd like to replay for a channel.
*/
struct channeltx *wallet_channeltxs_get(struct wallet *w, const tal_t *ctx,
u32 channel_id);

#endif /* LIGHTNING_WALLET_WALLET_H */