sql: rewrite scid string literals to use scid() for index support - #9430
vincenzopalazzo wants to merge 7 commits into
Conversation
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
6afaed2 to
c248e41
Compare
BOLT #1 says upon receiving a channel-scoped error the node MUST fail that channel. connectd only told lightningd about WIRE_ERROR when find_subd failed; if a dying channeld still existed the error was queued and never read, so the channel stayed CHANNELD_NORMAL. The test rolls l2's db back after a payment, freezes l1's channeld after reestablish, then lets l2 send "Awaiting unilateral close". It fails until the next commit. Reproduces: ElementsProject#9424 Reported-by: Leo Nash (@tankyleo) Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
channeld abort()s on WIRE_ERROR ("swallowed by connectd"), but we
only told lightningd when find_subd failed. If a subd existed we
enqueued the error; a dying channeld never read it, and a live
one would abort, so the channel stayed up.
Always tell lightningd via CONNECTD_PEER_SPOKE and do not enqueue.
handle_peer_spoke already permanently fails the channel.
Fixes: ElementsProject#9424
Changelog-Fixed: Protocol: fail the channel on a peer error even if channeld is exiting.
Reported-by: Leo Nash (@tankyleo)
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Andezion
left a comment
Andezion
left a comment
There was a problem hiding this comment.
The character by character copy loop (tal_append_fmt(&result, "%c", *p); p++;) for every non-quote byte is pretty "expensive" - each call goes through tal_append_fmt realloc/vsnprintf machinery for a single byte. Maybe we can find next ' with strchr and append the whole non-quoted span in one tal_append_fmt(&result, "%.*s", ...) call, mirroring what the function already does for the quoted spans??
peer_read() in the subd is what emits "peer_in WIRE_ERROR". Swallowing the error before enqueue means that line never appears, and the closing tests that wait on it time out. Log the inbound error in connectd and still do not hand it to channeld.
Short channel IDs were stored as TEXT strings (e.g., "735095x480x1") in
SQLite, which prevented efficient use of indexes on SCID columns. Change
storage to INTEGER (the u64 encoding), using a custom "SCID" column type
so the result-reading code can detect these columns and format them back
as "NNNxNNNxNNN" strings for backward-compatible JSON output.
Add two new SQL functions:
- scid('NNNxNNNxNNN') -> integer: for efficient WHERE clause filtering
- fmt_scid(integer) -> 'NNNxNNNxNNN': for formatting in SQL expressions
Fixes ElementsProject#8941
When users query with WHERE in_channel='735095x480x1', the string
literal is compared directly against the integer SCID column, forcing
SQLite to perform a full table scan even when an index exists.
Automatically rewrite scid string literals (matching NNNxNNNxNNN format)
to use the scid() function before passing the query to SQLite, so
'735095x480x1' becomes scid('735095x480x1'). This allows SQLite to
use indexes on SCID columns transparently.
Queries already using scid() explicitly are detected and left unchanged.
Changelog-Fixed: sql plugin now automatically translates short_channel_id string literals to integers for efficient index usage.
Fixes: ElementsProject#8941
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
c248e41 to
cc587d1
Compare
| span = p; | ||
|
|
||
| start = p + 1; | ||
| end = strchr(start, '\''); |
There was a problem hiding this comment.
strchr(start, '\') ends a literal at the first ', but SQL escapes a quote inside a string as ' '. For 'it''s 1x2x3', or 'a''1x2x3''', the parser loses track of where literals start and end. it can then insert scid(...) in the middle of a string, which gives a syntax error or a different query
| if (looks_like_scid(start, end - start)) { | ||
| /* Already wrapped: scid('735095x480x1'). Leave it. */ | ||
| bool already_wrapped = false; | ||
| if (p - query >= 5 && strncmp(p - 5, "scid(", 5) == 0) |
There was a problem hiding this comment.
in sql_scid_func(), if the argument is SQLITE_INTEGER, return it unchanged. Double wrapping then becomes harmless and the already_wrapped code can be removed. fmt_scid() should give an error for non-integer input
| /* Rewrite SQL query to wrap scid string literals with scid() function. | ||
| * This transforms '735095x480x1' into scid('735095x480x1') so that | ||
| * SQLite can use indexes on integer SCID columns. */ | ||
| static const char *rewrite_scid_literals(const tal_t *ctx, const char *query) |
There was a problem hiding this comment.
rewrite_scid_literals() does not know which column a literal is compared with. So WHERE label = '100x2x3' on invoices becomes label = scid('100x2x3'). That compares text with an integer, and the query silently returns nothing. The same happens with description, SELECT '1x2x3' and json_object('k', '1x2x3')
it gets worse because short_channel_id_from_str() uses sscanf("%ux%ux%hu") and never checks for trailing characters (bitcoin/short_channel_id.c at line 51). So '100x2x3-backup' also counts as an scid, gets rewritten, and is truncated to 100x2x3
maaaaybe we should not rewrite sql text, we can keep TEXT storage, or store integers, but document scid() as the way to filter, and do not rewrite queries. or require the whole literal to match the scid format exactly, with no trailing characters
This branch has not been deployed
2 participants
Confirmed still relevant on master (2026-08-12):
plugins/sql.chas noscid()handling, scid string literals still cannot use indexes.Summary
WHERE in_channel='735095x480x1', the string literal was compared directly against the integer SCID column, forcing SQLite into a full table scan even with an index presentjson_sql()passes user queries directly tosqlite3_prepare_v2()without translating scid string literals to integersNNNxNNNxNNNformat to use thescid()function before query execution, so'735095x480x1'becomesscid('735095x480x1')transparentlyFixes #8941
Changelog-Fixed: sql plugin now automatically translates short_channel_id string literals to integers for efficient index usage.
Test plan
SELECT * FROM forwards WHERE in_channel='735095x480x1'now use indexesscid('...')explicitly are left unchanged