sql: store short_channel_id as integer for efficient indexing#7
Closed
vincenzopalazzo wants to merge 311 commits intomasterfrom
Closed
sql: store short_channel_id as integer for efficient indexing#7vincenzopalazzo wants to merge 311 commits intomasterfrom
vincenzopalazzo wants to merge 311 commits intomasterfrom
Conversation
When a peer doesn't support OPT_SHUTDOWN_ANYSEGWIT, we fall back to P2WPKH for the shutdown script. For BIP86 wallets, we need to use bip86_pubkey for derivation (matching p2tr_for_keyidx), otherwise the resulting script won't be recognized after restart.
The signmessagewithkey RPC was failing for BIP86 (mnemonic-based) wallets because: 1. The wallet RPC was iterating through BIP32-derived addresses only, so it couldn't find BIP86-derived addresses. 2. The HSM's handle_bip137_sign_message always used bitcoin_key() (BIP32 derivation) regardless of wallet type.
listaddrs is dev only and used in tests so it's okay if we change the API here, the usage is by positional arguments in tests so we're okay. Also changing est_option_upfront_shutdown_script to handle both old hsmsecret and the newer mnemonic one.
The recover command checks if a node has already issued bitcoin addresses before allowing recovery. This check only looked at bip32_max_index, but with BIP86 wallets, newaddr() increments bip86_max_index instead. Also, the recover test asserted on hex but now it's asserting on codex32 instead. We should probably go in and fix the end point. @rustyrussell what do you think?
…ecret. Even though we don't do taproot addresses on elements yet, use the same scheme for simplicity and for future when we *do* do taproot. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
- Upgraded Fedora base image from 35 to 40. The existing Cargo failures were caused by the outdated Rust toolchain in Fedora 35. Cargo lockfile format v4 was introduced in Cargo 1.84 (December 2024), while Fedora 35 provides a Rust/Cargo version from roughly 2021–2022. As a result, the system Cargo could not parse modern Cargo.lock files, making it incompatible with current Rust projects. - Added missing build dependencies to the Dockerfile, most notably the protobuf compiler (protoc). Changelog-None: Upgraded Fedora version to 40 for reproducible build.
This caused issues when dependencies were updated do to cached images continuing to be used. Changelog-None
- Using environment variable `SOURCE_DATE_EPOCH` with fixed value will enforce a consistent timestamp for Fedora build. . Similar to Ubuntu fix in commit ElementsProject@490fb0f - Locked cargo version - Add `no-cache` to Fedora build Changelog-Fixed: Core lightning builds for Fedora on all systems are deterministic.
Changelog-Added: clnrest: add clnrest-register-path rpc method to register dynamic paths
…ure is disabled. Changelog-Fixed: Testing infrastructure no longer fails when logging output capture is disabled.
And sorted list with file name.
readme v2 API expects files to be .mdx compatible.
… binary diffs. The giant text lines make emacs ... run... slowly. Finally got around to fixing it to see that it had already been done, just not updated! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is important for the first xpay deprecation end-of-life! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The code had this as 26.03, but the documentation said 26.06. The usual deprecation period is a year, so I'm changing the documentation. Unfortunately the documentation (fe4d503) was updated separately from the code (afb54ff), so this wasn't obvious at the time! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means: 1. downgrade changes (we no longer fail due to node biases). 2. various deprecations no longer are Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We haven't announced websocket addresses for some time! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
… to use. I tried removing sign_penalty_to_us, but that comment is wrong: channeld uses that for the watchtower, so it stays (with updated comment). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…lc_tx" option. Everyone should be using the new name. Changelog-Removed: JSON-RPC: `listpeers` `features` array string "option_anchors_zero_fee_htlc_tx": use "option_anchors" (spec renamed it). Deprecated in 24.08. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: JSON-RPC: `decodepay` (use `decode`), deprecated v24.11. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: JSON-RPC: `close` `tx` and `txid` field (use `txs` and `txids`), deprecated v24.11. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…bjects, not names. This was changed in v0.9.2 (November 2020), with a comment saying to remove. But it turns out that the rust plugin support still uses the old method (found this by removing it and watching everything fail!). So simply undeprecate and document. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…onses. This was supposed to be removed in v24.05. Finally remove it now. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Removed: lightningd: `estimatefeesv1` support for older bcli plugins (deprecated v23.05, disabled by default v24.05).
We can actually be slow enough that we get a (60 second) MPP timeout
under valgrind:
```
# Restart, and make sure it's reconnected to l2.
l3.restart()
print(l2.rpc.listpeers())
wait_for(lambda: [p['connected'] for p in l2.rpc.listpeers()['peers']] == [True, True])
# Pay second part.
l1.rpc.sendpay(
route=route,
payment_hash=inv['payment_hash'],
amount_msat=1000,
bolt11=inv['bolt11'],
payment_secret=paysecret,
partid=2,
groupid=1,
)
> l1.rpc.waitsendpay(payment_hash=inv['payment_hash'], timeout=TIMEOUT, partid=1)
tests/test_pay.py:3352:
...
E pyln.client.lightning.RpcError: RPC call failed: method: waitsendpay, payload: {'payment_hash': 'd73e10b604f53afb05da052d1fc90c45269321169196e31b7ccc312eabc26557', 'timeout': 180, 'partid': 1}, error: {'code': 204, 'message': 'failed: WIRE_MPP_TIMEOUT (reply from remote)', 'data': {'created_index': 1, 'id': 1, 'payment_hash': 'd73e10b604f53afb05da052d1fc90c45269321169196e31b7ccc312eabc26557', 'groupid': 1, 'partid': 1, 'destination': '03cecbfdc68544cc596223b68ce0710c9e5d2c9cb317ee07822d95079acc703d31', 'amount_msat': 500, 'amount_sent_msat': 501, 'created_at': 1771551338, 'status': 'pending', 'bolt11': 'lnbcrt10n1p5e0wnfsp5vpgn0adhmyjecp47yfc6dtefett7d3qqzh4j908ane7tezyy0evqpp56ulppdsy75a0kpw6q5k3ljgvg5nfxggkjxtwxxmuescja27zv4tsdq9d9h8vxqyjw5qcqp9rzjqvuytqpdyk6wqaxvl47d3vee5swuwklej79qxjqqg394r4ptqaue5qqqvuqqqqgqqqqqqqqpqqqqqzsqqc9qxpqysgq6ur4cclfhnzs0tgenmxhvx8glw3q9eylp3pdck73x58cukwtfxj5zjg6g0a4p4plgj6qd0s0uxkuntzcn4l6vvndgrjj7gndma3t7pcqmr9q3j', 'erring_index': 2, 'failcode': 23, 'failcodename': 'WIRE_MPP_TIMEOUT', 'erring_node': '03cecbfdc68544cc596223b68ce0710c9e5d2c9cb317ee07822d95079acc703d31', 'erring_channel': '103x1x0', 'erring_direction': 0, 'raw_message': '0017'}}
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
ChatGPT suggested this for sanitizer runs, but valgrind gains a little too. Sanitizer before: 74.1-98.9(84.5+/-7.3) minutes Sanitizer after: 11.6-15.2(13.3+/-1.2) minutes Valgrind before: 43.3-54.1(46.8+/-3.8) minutes Valgrind after: 40.9-49.4(43.4+/-2.5) minutes Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Given the new timings, this should even things out a little. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It takes 1h43m, and it's our "basic" integration test. So split 6 ways to get faster results, then if that passes do the other tests (Postgres, clang, Liquid, splicing, dual-funding). Now they take about 20 minutes each. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The other integration tests are taking under an hour, and this is taking 1hr 30m. The downside is that backtraces are far less clear, but that's why the main run uses the full debug version. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Sometimes we don't know the short_channel_id yet (e.g the channel has not been broadcasted), but we still know the channel_id from the funding transaction. Add a channel_id filter to listpeerchannels and clarify that id, short_channel_id and channel_id are mutually exclusive. Changelog-Added: `listpeerchannels` now accepts a `channel_id` filter, for cases where the `short_channel_id` is not known yet. Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
Changelog-None
Changelog-None
It's timing out after 2 hours sometimes: this now make it finish in 53 minutes. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…f random delays. Sometimes this times out after 30 minutes. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Usually downloading and installing takes 90 seconds. But sometimes it takes an hour! Use caching for this, to keep it consistent. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
.github/scripts/setup.sh does this already, *and* it uses the cache now. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I noticed this line causing a delay; ChatGPT thinks NSS name lookup. I've removed the useradd line altogether. I also install eatmydata first, to try to speed the other installs. This drops the install step from 1m45 to about 30 seconds. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
On the GH action view I can only see "Compile CLN compile-c" which is not enough! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Only do it when the tcp_capture fixture is invoked, which is only for one test. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
287abfb "ci: Add a simple plugin to report test results to our falkiness tracker" slowed our integration tests to a crawl: out 6-way gcc integration tests should have taken 20 minutes, but were taking 1hr 24 minutes, and some runs were timing out altogether. I cannot reach the server, seems it is down. But tracking flakiness is rarely useful, so I am disabling this. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: new plugin currencyrate to provide `currrencyconvert` API
…ate-add/disable-source. @daywalker90's was a little *too* faithful a reimplementation of the Python one! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Instead of fetching prices on every request with a very short cache TTL we now start a background task instead with a way longer cache TTL. The background task fetches prices only from one source by default. If the price has changed much we fetch from different sources until two sources agree on a price. The background task exits if there wasn't a request for that currency in some time.
Changelog-None
vincenzopalazzo
force-pushed
the
claude/eloquent-meitner
branch
2 times, most recently
from
922b1e1 to
8627c69
Compare
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
vincenzopalazzo
force-pushed
the
claude/eloquent-meitner
branch
from
8627c69 to
84562df
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FIELD_SCIDmapped to SQL typeTEXT, so comparisons and indexes operated on text representationsSCIDcolumn type, preserving backward-compatibleNNNxNNNxNNNstring output in JSON responses viasqlite3_column_decltype()detectionscid()andfmt_scid()SQL functions for string-to-integer and integer-to-string conversion within queriesFixes ElementsProject#8941
Test plan
test_sqlpasses with updated schema type mapping ('short_channel_id': 'SCID') andfmt_scid()usage injson_object()expressionsNNNxNNNxNNNformat inSELECTresults (backward compatible)WHERE in_channel = scid('1x2x3')uses integer index efficiently🤖 Generated with Claude Code