Repository navigation
fix: gate WASM CLI-spec registration on unix; fix all-features test compile; monotonic RPC rate limiter - #28
Merged
Conversation
…t types ModuleManager::ipc_server only exists on unix, but the WASM load path read it ungated, so blvm-node failed to compile for x86_64-pc-windows-gnu with wasm-modules (E0609/E0282; blvm run 39185005040, Windows blvm-sdk experimental). Gate the registration like the other ipc_server uses. tests/storage_tests.rs: the witness-compression test passed a witness stack one level too shallow for store_witness(&[Vec<Witness>]), and the utxo-compression test used a u32 value and an unwrapped UTXO. Build the correct shapes (one tx -> one input -> one stack; i64 value; Arc<UTXO>); no API change.
…crate c161636 dropped uuid from the manifest in favour of utils::new_request_id, but payment_rest_integration_tests still imported uuid::Uuid, so the test failed to compile with ctv+bip70-http+rest-api (any --all-features --tests build). It was hidden behind the storage_tests failure, which stops cargo first.
RpcRateLimiter refilled from current_timestamp(), whole wall-clock Unix seconds. Any wait that straddled a second boundary was credited a full second of tokens, so 2s could count as 3s and a request right after the burst could be refilled after a few microseconds. That is the flake in rpc::rest::tests::security_tests::test_rate_limiting_time_window (run 39405496449). Wall-clock steps (NTP, manual) also moved the bucket. The limiter now uses tokio::time::Instant and credits elapsed nanoseconds exactly: elapsed_ns * rate plus a carried sub-token remainder (units of 1e-9 token, u128 math), whole tokens into the bucket, remainder carried. Tokens stay clamped to burst_limit; a full bucket or rate 0 banks nothing. Burst, rate and batch (n > burst) semantics are unchanged, and every caller still builds the limiter with the same (burst, rate) as before. last_refill() now returns the monotonic Instant of the last check, and cleanup_stale_limiters() compares against Instant::now() with the same one-hour threshold. Tests: limiter tests that slept now run on a paused tokio clock with advance() (tokio test-util as a dev-dependency). New unit tests cover a real-clock second-boundary reproduction, wall-clock independence, exact carry over 1000 x 1ms and with interleaved consumption, the burst cap, huge elapsed/rate without overflow, and monotonic stale eviction. The ignored QUIC 429 smoke test uses rate 0 so it no longer depends on timing.
blvm-consensus docs/VERIFICATION.md (ed2fac0) counts 3 of 182 anchors as mutation-validated; create_new_block has no #[spec_locked]. Say the mining functions are tested, call spec-locked network code annotated and Z3-checked, and say the SECURITY.md item is property tests. Comments and docs only.
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
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.
Fixes the blvm-node compile errors in blvm's v0.1.62 Official Release (run 39185005040, job 117845486301). This PR is based on
mainand is independent of #27.1. Windows lib build (
src/module/manager.rs)error[E0609]: no field 'ipc_server' on type '&mut ModuleManager'atblvm-node-0.1.73/src/module/manager.rs:724, followed byE0282.--all-features, which brings inblvm-node/wasm-modules), soblvm-sdk experimentalfailed for Windows.ipc_server(decl :39-40) and theipc_server()accessor (:315) are both#[cfg(unix)]. The WASM load path read the field ungated.#[cfg(unix)]on thatif let, the same pattern as the other three uses (:457, :569, :898).cli_specis then unused.[lints.rust] unused_variables = "allow"covers that, andcargo checkfor the Windows target shows no new warning.src/.2.
tests/storage_tests.rs(cargo build --release --all-features --testsin the same run)store_witness(&[Vec<Witness>])was givenVec<Vec<Vec<u8>>>. It now builds tx → input → stack:Vec<Vec<Witness>> = vec![vec![vec![vec![..]]]].u32value wherei64is required (nowi64::from(i)), and :1310 inserted aUTXOwhereArc<UTXO>is required (nowArc::new). No API change.3.
tests/payment_rest_integration_tests.rs(not in the original ask; found by--keep-going)ctv + bip70-http + rest-apiit still diduse uuid::Uuid, but c161636 removeduuidin favour ofblvm_node::utils::new_request_id. Cargo stopped at storage_tests first, so the release log never showed this error.new_request_id(), which uses the crate's own helper and adds no dependency.test_rest_unvault_vault, 400 instead of 200. The request id is opaque and has nothing to do with it. 9 of 10 pass.cargo testdoesn't enablectv, so CI isn't affected. In blvm's release workflow the test build and test run areif ! cmd | tee, so they never fail the job, and the log reported "Tests compiled successfully" even though the build failed.Run locally (sibling crates at their
origin/main)cargo check -j 2 --lib --target x86_64-pc-windows-gnu --no-default-features --features <blvm-sdk's node set>,wasm-modules:main: E0609 + E0282, matching the log.--all-featurescan't run on Windows here:secp-gpuneeds nvcc.cargo check -j 2 --tests --keep-goingwith every feature exceptsecp-gpu/rocksdb/erlay/tidesdb(those need CUDA, libclang, or a C toolchain the box lacks) passes.cargo test --test storage_tests compression(sled+compression): 5/5 pass, including both fixed tests.4. RPC rate limiter refills from a monotonic clock (d5079c8)
rpc::rest::tests::security_tests::test_rate_limiting_time_windowflaked on main f82d8c4. Run 39405496449 failed atsecurity_tests.rs:266(assertion failed: !auth.check_rate_limit(&user_id).await), and the rerun 39408183650 passed.origin/mainis merged into this branch (d52c907, a clean merge) before this commit.Cause:
RpcRateLimiter(src/rpc/auth_impl.rs) refilled fromcurrent_timestamp(), which is whole wall-clock Unix seconds (last_refill: u64,elapsed * rate).Fix: the limiter uses
tokio::time::Instantand credits elapsed time exactly in integers.elapsed_ns * rate(u128) plus a carried sub-token remainder (units of 1e-9 token, < 1e9). Whole tokens go into the bucket and the remainder is carried.burst_limit. A full bucket orrate == 0banks nothing.new(burst, rate),check_and_consume[_n](including then > burstreject) andtokens_remaining()are unchanged.last_refill(), which now returns the monotonicInstantof the last check instead of Unix seconds. Its only user iscleanup_stale_limiters(), which keeps the same one-hour threshold onInstant::now().Callers get the same limits as before. No construction site changed its
(burst, rate)arguments:RpcServer::build_auth_manager→with_rate_limits(required, rate_limit_burst, rate_limit_rate), fromRpcAuthConfig(TOML/env; defaults 100 / 10 inconfig/rpc.rs)RpcAuthManager::newdefault(100, 10)set_user_rate_limit, per-methodset_method_rate_limitcheck_ip_rate_limit_with_endpoint[_n]: the default limit, halved when auth is required)check_rate_limit[_n]/check_rate_limit_with_endpoint[_n](user limiters)Both RPC servers (
rpc/server.rs,rpc/rest/server.rs) only call these checks. The peer and module limiters (network/peer_manager.rs,module/api/hub.rs) are separate types and aren't touched.Tests:
#[tokio::test(start_paused = true)]andtokio::time::advance. tokiotest-utilwas added as a dev-dependency at the same=1.48.0. Files:src/rpc/rest/tests/security_tests.rs: 3 teststests/rpc_rate_limiting_tests.rs: all of them.test_rate_limiter_high_rateis no longer#[ignore].tests/rpc_auth_security_tests.rs: 2tests/rpc_auth_comprehensive_tests.rs: 5. The two stub tests now assert refill and the cap.quic_rpc_rate_limit_rejection(needsquinn) uses rate 0, so its 429 doesn't depend on timing. It's not compiled under default features and I didn't run it.auth_impl.rs:advance(1ms)gives exactlyfloor(rate * t)at every step, and exactlyrateafter 1s, for rates 1/3/7/10/999/1000/1001rate = u32::MAXu32::MAXburst and rate with ~100 years elapsed: no overflow or panic, and a partial refill is exactEvidence for item 4 (local, sibling patches stripped as CI does, fresh lock: blvm-protocol 0.1.34, blvm-consensus 0.1.64)
auth_impl.rs, applied withcp, plus the new real-clock and wall-clock tests. The new boundary test, the wall-clock test andtest_rate_limiting_time_window(:266) failed on all 50 of 50 runs.rate_limittests plusrpc_rate_limiting_tests(10),rpc_auth_comprehensive_tests(17) andrpc_auth_security_tests(15).cargo test -j 2 --lib: 1468 passed, 2 ignoredcargo test -j 2 --features stratum-v2 --lib: 1469 passed, 2 ignoredcargo fmt --checkpasses, and both CI clippy invocations are clean.cargo check -j 2 --target x86_64-pc-windows-gnupasses for default features and for--tests.cargo testdidn't finish locally because the box ran out of disk while linking. CI runs it.5. Docs: verification claims narrowed (7ade033, comments and docs only)
This matches blvm-consensus
docs/VERIFICATION.md(ed2fac0), where 3 of 182 anchors are mutation-validated.create_new_blockhas no#[spec_locked].src/rpc/mining.rs:4: "Uses formally verified blvm-consensus mining functions." now says the functions are covered by tests, are not formally verified, and links VERIFICATION.md.src/module/traits.rs:495: "same formally verified consensus function" now reads "same tested blvm-consensus mining function".SECURITY.md:175: "Formal verification of critical paths (property tests ...)" now reads "Property tests for node invariants on critical paths (testing, not formal verification)".docs/NETWORK_VERIFICATION.md: