nik/liana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge #499: sqlite: prevent races when updating the next derivation index

Browse Source 
039b82d9b3e11786f09c761a6d0a6d137cdff8ad sqlite: prevent races when updating the next derivation index (Antoine Poinsot)

Pull request description:

  You can check the unit test triggering the race fails on master. I'm pretty sure it's the reason for the address generation bug i experienced yesterday when testing.

ACKs for top commit:
  edouardparis:
    utACK 039b82d9b3e11786f09c761a6d0a6d137cdff8ad

Tree-SHA512: e245d8732346731c5a98223363c9d2e77f3a568e046055007162167f7a7209cc0e9d0f57c7219e469788ff73c570962396c4da0e056c790ffffbd953c75ab3ee
This commit is contained in:
Antoine Poinsot 2023-05-12 13:02:51 +02:00
commit ec55348693
No known key found for this signature in database
GPG Key ID: E13FC145CD3F4304
1 changed files with 19 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/database/sqlite/mod.rs
View File

@ -243,6 +243,14 @@ impl SqliteConn {
.pop()
.expect("There is always a row in the wallet table");
// Make sure we don't set a lower derivation index. This can happen since the
// derivation is set outside the atomic transaction. So there may be a race between say
// the Bitcoin poller thread and the JSONRPC commands thread.
if (change && index <= db_wallet.change_derivation_index) || (!change && index <= db_wallet.deposit_derivation_index) {
// It was already set at a higher index.
return Ok(());
}
// First of all set the derivation index
let index_u32: u32 = index.into();
if change {
@ -1015,6 +1023,17 @@ CREATE TABLE spend_transactions (
let db_addr = conn.db_address(&addr).unwrap();
assert_eq!(db_addr.derivation_index, look_ahead_index.into());
}
// Suppose the latest change derivation index was set to 52 above by the commands
// thread. Suppose concurrently the Bitcoin poller thread queried the DB for the
// latest derivation just before it happened, got 2 as a response, and then increased
// the derivation index to -say- 7 after noticing a new change output paying to the
// address at derivation index 6. It's absolutely possible and the only way to prevent
// this is to make sure *within* the atomic DB transaction that we will never decrease
// the derivation index. Make sure we actually perform this check (note it would only
// crash during the second call).
conn.set_derivation_index(7.into(), true, &secp);
conn.set_derivation_index(8.into(), true, &secp);
}
fs::remove_dir_all(tmp_dir).unwrap();