Skip to content

wasm: Route clock reads through a time module - #1122

Open
benthecarman wants to merge 5 commits into
lightningdevkit:mainfrom
benthecarman:wasm-time
Open

wasm: Route clock reads through a time module#1122
benthecarman wants to merge 5 commits into
lightningdevkit:mainfrom
benthecarman:wasm-time

Conversation

@benthecarman

@benthecarman benthecarman commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

First step toward wasm32 support: stop reading the system clock directly, since SystemTime::now() panics on wasm32-unknown-unknown.

  • Add ldk_node::time. All clock reads go through it, and hosts without a
    system clock can install a TimeProvider.
  • Use LDK and BDK's explicit-time APIs instead of the ones that read the clock
    internally.
  • Move LDK's std features behind a default ldk-std feature, and
    lightning-net-tokio behind a default net-tokio feature. Both are temporary
    scaffolding for the port.
  • Add a CI job that bans direct clock reads and checks that LDK's std/time
    features are off when default features are disabled.

Native behavior should be unchanged.

@ldk-reviews-bot

ldk-reviews-bot commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@benthecarman benthecarman added this to the 0.9 milestone Oct 1, 2026
@ldk-reviews-bot
ldk-reviews-bot requested a review from tnull October 1, 2026 20:00
tnull
tnull reviewed Oct 6, 2026
View reviewed changes

@tnull tnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

Comment thread src/payment/forwarding_store.rs
now,
FORWARDED_PAYMENT_AGGREGATION_BUCKET_SIZE_SECS,
);
// Tokio deadlines must use the runtime clock, whose origin may differ from our provider.

@tnull tnull Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm, so this will panic if exercised in WASM?

@benthecarman benthecarman Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For now, will do tokio related stuff in the future

Comment thread src/lib.rs Outdated
pub mod probing;
mod runtime;
mod scoring;
pub mod time;

@tnull tnull Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why pub?

@benthecarman benthecarman Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no longer with the builder change

Comment thread src/time.rs

impl std::error::Error for TimeProviderAlreadyInitialized {}

static TIME_PROVIDER: OnceLock<Arc<dyn TimeProvider>> = OnceLock::new();

@tnull tnull Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are we positive to make this a OnceLock? If we made it a 'regular' parameter to Node settable via the Builder, it could be useful, eg., for testing purposes to allow some nodes to experience clock drift etc?

@benthecarman benthecarman Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed it to be a part of the Builder but it goes into the OnceLock. If it's just a part of the Node it percolates everywhere and gets kinda ugly

Comment thread .clippy.toml Outdated
@@ -1 +1,14 @@
msrv = "1.63.0" No newline at end of file
msrv = "1.63.0"

@tnull tnull Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is outdated. Mind updating in a commit since you're already here?

@benthecarman benthecarman Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

benthecarman and others added 5 commits October 6, 2026 19:32
@benthecarman @claude
Centralize clock reads in a time module
b26e469 
Keep node wall and monotonic clock reads behind one provider in the time
module. Use native clocks by default and let embedding hosts supply one
through the builder. The provider is process-wide and can't be replaced
once in use, so monotonic measurements share one origin. Nodes can
share a provider, but building with a different one fails.

Drop chrono's clock feature, as UTC timestamps now come from the
provider.

Ban direct platform clock reads with clippy and run the check in a
dedicated CI job across the native and UniFFI feature sets, so new code
stays wasm32-safe while the rest of the port lands.

AI assistance: Claude Opus 5.5 and OpenAI Codex.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@benthecarman @claude
Use explicit time in dependency APIs
d40ecec 
Supply node time to BDK scans, liquidity, invoice timestamps, gossip
snapshots, and UniFFI expiry checks through existing explicit-time APIs.
Preserve freshness and expiry validation, and report a pre-epoch clock
as an LSPS2 invoice creation error.

Disable transaction-sync's optional timing logs, which duplicate the
node's own sync duration logs, so it no longer reads the clock itself.

Ban the dependency APIs that read the platform clock internally, so
call sites keep using the explicit-time variants.

AI assistance: Claude Opus 5.5 and OpenAI Codex.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@benthecarman @claude
Gate LDK std behind an ldk-std feature
572601e 
LDK's std feature reads the system clock internally (channel manager,
gossip, offers, payment retries, liquidity), which wasm32 can't do.
Move it and liquidity's time feature behind a default ldk-std feature,
so code can switch to LDK's explicit-time APIs when it is disabled.

The chain sources and filesystem storage pull in LDK crates that
require std, so they enable ldk-std. Native builds resolve the same LDK
features as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@benthecarman @claude
Gate the tokio peer transport behind a feature
d92737e 
lightning-net-tokio enables LDK's default features, which turns std
back on even when ldk-std is disabled. Move it behind a default
net-tokio feature so builds without ldk-std are free of LDK's internal
clock reads.

Without net-tokio, a placeholder socket type stands in for the
transport. It has no values, so no connection can be created with it:
connecting to peers fails and listening addresses are rejected at build
time. This leaves the seam for a wasm32 transport later.

Restore the CI check that disabling default features leaves no LDK
clock features enabled, and lint a build without net-tokio so the
placeholder keeps compiling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@benthecarman @claude
Update clippy MSRV to 1.85
41be68a 
The crate's minimum supported Rust version is 1.85, but .clippy.toml
still targeted 1.63, so clippy applied lint rules for the wrong
toolchain.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tnull tnull tnull left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.9

Development

Successfully merging this pull request may close these issues.

3 participants

@benthecarman @ldk-reviews-bot @tnull