Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions docs/release-notes/release-notes-next.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@

#### Maintenance

* Reserved distinct static-address receive and change key families and
persisted deposit address ownership metadata in preparation for multi-address
Static Address Loop In support.
[PR #1210](https://github.com/lightninglabs/loop/pull/1210)

* Updated the gRPC dependency to v1.83.1.

* Updated the Taproot Assets dependency to v0.8.1; asset conversions that
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE deposits DROP COLUMN static_address_id;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
ALTER TABLE deposits ADD static_address_id INT REFERENCES static_addresses(id);

UPDATE deposits
SET static_address_id = (
SELECT id FROM static_addresses ORDER BY id ASC LIMIT 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 F1 (Major) — Backfill assigns every old deposit to one address · loopdb/sqlc/migrations/000022_deposit_static_address_id.up.sql:5

The migration assigns every pre-existing deposit to the lowest-id row in static_addresses. If a client ever created more than one static address before this migration runs, deposits belonging to the second address are permanently recorded as owned by the first, and every subsequent read hands spending code the wrong client/server pubkeys and expiry through the new AddressParams.

Why this matters

The subselect SELECT id FROM static_addresses ORDER BY id ASC LIMIT 1 has no way to tell which address produced a given deposit — deposits stores only timeout_sweep_pk_script, not the address pkScript — and the EXISTS guard only checks that at least one address row exists, not that exactly one does. The mis-assignment is also unrecoverable through the repair path this PR adds: SetAllNullDepositsStaticAddressID (loopdb/sqlc/queries/static_address_deposits.sql) and BatchSetStaticAddressID (staticaddr/deposit/sql_store.go) both filter on static_address_id IS NULL, so a row that was wrongly filled is never revisited. Gating the backfill on (SELECT COUNT(*) FROM static_addresses) = 1 and leaving the column NULL otherwise keeps the ambiguous case in the hands of application code, which can derive scripts and match them against each deposit. I cannot tell from the loaded context whether the client can currently create more than one static address; if it provably cannot, this reduces to a hardening request.

)
WHERE static_address_id IS NULL
AND EXISTS (SELECT 1 FROM static_addresses);
1 change: 1 addition & 0 deletions loopdb/sqlc/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions loopdb/sqlc/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 44 additions & 10 deletions loopdb/sqlc/queries/static_address_deposits.sql
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ INSERT INTO deposits (
confirmation_height,
timeout_sweep_pk_script,
expiry_sweep_txid,
finalized_withdrawal_tx
finalized_withdrawal_tx,
static_address_id
) VALUES (
$1,
$2,
Expand All @@ -16,7 +17,8 @@ INSERT INTO deposits (
$5,
$6,
$7,
$8
$8,
$9
);

-- name: UpdateDeposit :exec
Expand All @@ -43,29 +45,56 @@ INSERT INTO deposit_updates (

-- name: GetDeposit :one
SELECT
*
d.*,
sa.client_pubkey client_pubkey,
sa.server_pubkey server_pubkey,
sa.expiry expiry,
sa.client_key_family client_key_family,
sa.client_key_index client_key_index,
sa.pkscript pkscript,
sa.protocol_version protocol_version,
sa.initiation_height initiation_height
FROM
deposits
deposits d
LEFT JOIN static_addresses sa ON sa.id = d.static_address_id
WHERE
deposit_id = $1;

-- name: DepositForOutpoint :one
SELECT
*
d.*,
sa.client_pubkey client_pubkey,
sa.server_pubkey server_pubkey,
sa.expiry expiry,
sa.client_key_family client_key_family,
sa.client_key_index client_key_index,
sa.pkscript pkscript,
sa.protocol_version protocol_version,
sa.initiation_height initiation_height
FROM
deposits
deposits d
LEFT JOIN static_addresses sa ON sa.id = d.static_address_id
WHERE
tx_hash = $1
AND
out_index = $2;

-- name: AllDeposits :many
SELECT
*
d.*,
sa.client_pubkey client_pubkey,
sa.server_pubkey server_pubkey,
sa.expiry expiry,
sa.client_key_family client_key_family,
sa.client_key_index client_key_index,
sa.pkscript pkscript,
sa.protocol_version protocol_version,
sa.initiation_height initiation_height
FROM
deposits
deposits d
LEFT JOIN static_addresses sa ON sa.id = d.static_address_id
ORDER BY
id ASC;
d.id ASC;

-- name: GetLatestDepositUpdate :one
SELECT
Expand All @@ -76,4 +105,9 @@ WHERE
deposit_id = $1
ORDER BY
update_timestamp DESC
LIMIT 1;
LIMIT 1;

-- name: SetAllNullDepositsStaticAddressID :exec
UPDATE deposits
SET static_address_id = $1
WHERE static_address_id IS NULL;
14 changes: 12 additions & 2 deletions loopdb/sqlc/queries/static_addresses.sql
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
-- name: AllStaticAddresses :many
SELECT * FROM static_addresses;
SELECT * FROM static_addresses
ORDER BY id ASC;

-- name: GetStaticAddress :one
SELECT * FROM static_addresses
WHERE pkscript=$1;

-- name: GetStaticAddressID :one
SELECT id FROM static_addresses
WHERE pkscript=$1;

-- name: CreateStaticAddress :exec
INSERT INTO static_addresses (
client_pubkey,
Expand All @@ -24,4 +29,9 @@ INSERT INTO static_addresses (
$6,
$7,
$8
);
);

-- name: GetLegacyAddress :one
SELECT * FROM static_addresses
ORDER BY id ASC
LIMIT 1;
Loading
Loading