diff --git a/.github/workflows/check_lint_build.yaml b/.github/workflows/check_lint_build.yaml index 0052013..7891a3e 100644 --- a/.github/workflows/check_lint_build.yaml +++ b/.github/workflows/check_lint_build.yaml @@ -6,11 +6,25 @@ on: tags: - '*' workflow_dispatch: + inputs: + base_branch: + description: "Base branch to diff against (only used when cargo_deny_check=auto)" + type: string + default: master + cargo_deny_check: + description: "Run or skip the `cargo deny` step" + type: choice + options: + - auto + - force-run + - force-skip + default: auto name: Check, Lint, Build env: CARGO_TERM_COLOR: always + ELECTRS_VERSION: v3.2.0 jobs: check-lint: @@ -27,11 +41,23 @@ jobs: toolchain: nightly components: rustfmt, clippy - - name: Rust Cache + - name: Restore Rust cache + id: restore-rust-cache + uses: Swatinem/rust-cache@v2.9.1 + with: + cache-all-crates: "true" + prefix-key: "v0-rust-nightly" + save-if: "false" + shared-key: debug-x86_64-unknown-linux-gnu + + - name: Save Rust cache + if: steps.restore-rust-cache.outputs.cache-hit != 'true' uses: Swatinem/rust-cache@v2.9.1 with: - cache-bin: false - shared-key: x86_64-unknown-linux-gnu + cache-all-crates: "true" + lookup-only: "true" + prefix-key: "v0-rust-nightly" + shared-key: debug-x86_64-unknown-linux-gnu - name: Rustfmt run: cargo fmt --all -- --check @@ -42,13 +68,56 @@ jobs: - name: Clippy run: cargo clippy --all-targets --all-features + - name: Check changed files + id: changed-files + if: github.event_name == 'pull_request' || (github.event_name == 'workflow_dispatch' && inputs.cargo_deny_check == 'auto') + uses: dorny/paths-filter@v4 + with: + filters: | + cargo_lock: + - 'Cargo.lock' + deny_toml: + - 'deny.toml' + - name: Deny + if: >- + (github.event_name == 'pull_request' && (steps.changed-files.outputs.cargo_lock == 'true' || steps.changed-files.outputs.deny_toml == 'true')) || + (github.event_name == 'workflow_dispatch' && inputs.cargo_deny_check == 'force-run') || + (github.event_name == 'workflow_dispatch' && inputs.cargo_deny_check == 'auto' && (steps.changed-files.outputs.cargo_lock == 'true' || steps.changed-files.outputs.deny_toml == 'true')) uses: EmbarkStudios/cargo-deny-action@v2 - + + cache-electrs: + name: Build electrs if not cached + runs-on: ubuntu-latest + steps: + - name: Check cache + id: check-cache + uses: actions/cache/restore@v6 + with: + lookup-only: true + path: ${{ runner.temp }}/electrs/target/release/electrs + key: electrs-${{ env.ELECTRS_VERSION }} + + - name: Build electrs + if: steps.check-cache.outputs.cache-hit != 'true' + run: | + git clone --branch ${{ env.ELECTRS_VERSION }} --depth 1 https://github.com/mempool/electrs.git ${{ runner.temp }}/electrs + pushd ${{ runner.temp }}/electrs + cargo build --locked --release + popd + + - name: Save cache + if: steps.check-cache.outputs.cache-hit != 'true' + uses: actions/cache/save@v6 + with: + path: ${{ runner.temp }}/electrs/target/release/electrs + key: electrs-${{ env.ELECTRS_VERSION }} + + integration-test: name: Integration test runs-on: ubuntu-latest - needs: [check-lint] + needs: [cache-electrs, check-lint] steps: - name: Download latest bitcoin-patched run: | @@ -81,37 +150,33 @@ jobs: rm -r bip300301-enforcer-latest-x86_64-unknown-linux-gnu chmod +x bip300301-enforcer popd - - name: Checkout electrs - run: | - pushd .. - git clone https://github.com/mempool/electrs.git - popd - - name: Install latest nightly toolchain - uses: dtolnay/rust-toolchain@master - with: - toolchain: nightly - target: x86_64-pc-windows-gnu - - name: Rust Cache (electrs) - uses: Swatinem/rust-cache@v2.9.1 + + - name: Restore electrs from cache + uses: actions/cache/restore@v6 with: - cache-bin: false - prefix-key: "v0-rust-electrs" - workspaces: ../electrs -> target - - name: Install electrs - run: | - pushd ../electrs - cargo build --locked --release - popd + fail-on-cache-miss: true + path: ${{ runner.temp }}/electrs/target/release/electrs + key: electrs-${{ env.ELECTRS_VERSION }} + - uses: actions/checkout@v6 with: submodules: "recursive" + + - name: Install latest nightly toolchain + uses: dtolnay/rust-toolchain@master + with: + toolchain: nightly + - name: Rust Cache uses: Swatinem/rust-cache@v2.9.1 with: - cache-bin: false - shared-key: x86_64-unknown-linux-gnu + cache-all-crates: "true" + prefix-key: "v0-rust-nightly" + shared-key: debug-x86_64-unknown-linux-gnu + - name: Build (debug) - run: cargo build + run: cargo build --bin plain_bitnames_app --example integration_tests + - name: Run integration tests id: runIntegrationTests run: | @@ -119,9 +184,9 @@ jobs: export BITCOIND='../bitcoin-patched-bins/bitcoind' export BITCOIND_UNPATCHED='../bitcoin-unpatched-bins/bitcoind' export BITCOIN_CLI='../bitcoin-patched-bins/bitcoin-cli' - export ELECTRS='../electrs/target/release/electrs' + export ELECTRS='${{ runner.temp }}/electrs/target/release/electrs' export BITNAMES_APP='target/debug/plain_bitnames_app' - cargo run --example integration_tests + target/debug/examples/integration_tests test-build-release: strategy: @@ -150,6 +215,10 @@ jobs: contents: write timeout-minutes: 20 steps: + - name: Install windows-specific deps + run: sudo apt install mingw-w64 + if: ${{ matrix.name == 'x86_64-pc-windows-gnu' }} + - uses: actions/checkout@v6 with: submodules: recursive @@ -163,12 +232,9 @@ jobs: - name: Rust Cache uses: Swatinem/rust-cache@v2.9.1 with: - cache-bin: false - key: ${{ matrix.name }} - - - name: Install windows-specific deps - run: sudo apt install mingw-w64 - if: ${{ matrix.name == 'x86_64-pc-windows-gnu' }} + cache-all-crates: "true" + prefix-key: "v0-rust-nightly" + shared-key: ${{ matrix.name }} - name: Test run: cargo test --tests diff --git a/.github/workflows/clean-gh-actions-caches-closed-pr-branch.yaml b/.github/workflows/clean-gh-actions-caches-closed-pr-branch.yaml new file mode 100644 index 0000000..928ef53 --- /dev/null +++ b/.github/workflows/clean-gh-actions-caches-closed-pr-branch.yaml @@ -0,0 +1,31 @@ +name: Evict GitHub actions caches for closed/merged PR branches +on: + pull_request: + types: + - closed + workflow_dispatch: + +jobs: + cleanup: + permissions: + actions: write + runs-on: ubuntu-latest + steps: + - name: Cleanup + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + BRANCH: refs/pull/${{ github.event.pull_request.number }}/merge + run: | + echo "Fetching list of cache keys" + BRANCH_CACHE_KEYS=$(gh cache list --ref $BRANCH --json id --jq '.[].id') + + ## Setting this to not fail the workflow while deleting cache keys. + set +e + echo "Deleting caches..." + for CACHE_KEY in $BRANCH_CACHE_KEYS + do + echo "Deleting cache ${CACHE_KEY}" + gh cache delete $CACHE_KEY + done + echo "Done" diff --git a/.github/workflows/clean-gh-actions-caches-nightly-rust.yaml b/.github/workflows/clean-gh-actions-caches-nightly-rust.yaml new file mode 100644 index 0000000..eced672 --- /dev/null +++ b/.github/workflows/clean-gh-actions-caches-nightly-rust.yaml @@ -0,0 +1,31 @@ +name: Evict stale nightly Rust caches +on: + schedule: + # every 12 hours + - cron: "0 */12 * * *" + workflow_dispatch: + +jobs: + cleanup: + permissions: + actions: write + runs-on: ubuntu-latest + steps: + - name: Delete nightly Rust caches older than 48h + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + run: | + CUTOFF=$(date -u -d '48 hours ago' +%s) + + CACHE_IDS=$(gh cache list --repo "$REPO" --limit 500 \ + --json id,key,createdAt | + jq -r --argjson cutoff "$CUTOFF" \ + '.[] | select(.key | startswith("v0-rust-nightly")) + | select((.createdAt | sub("\\.[0-9]+"; "") | fromdateiso8601) < $cutoff) + | .id') + + for CACHE_ID in $CACHE_IDS; do + echo "Deleting cache $CACHE_ID" + gh cache delete "$CACHE_ID" --repo "$REPO" + done diff --git a/Cargo.lock b/Cargo.lock index 5efc824..9926e4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -324,7 +324,7 @@ dependencies = [ "objc2-foundation 0.3.2", "parking_lot", "percent-encoding", - "windows-sys 0.59.0", + "windows-sys 0.52.0", "x11rb", ] @@ -1619,9 +1619,9 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.18" +version = "0.9.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" dependencies = [ "crossbeam-utils", ] @@ -5416,6 +5416,7 @@ dependencies = [ "serde_with", "sneed", "strum 0.28.0", + "temp-dir", "thiserror 2.0.18", "tiny-bip39", "tokio", @@ -5926,15 +5927,6 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" -[[package]] -name = "quick-xml" -version = "0.38.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b66c2058c55a409d601666cffe35f04333cf1013010882cec174a7467cd4e21c" -dependencies = [ - "memchr", -] - [[package]] name = "quick-xml" version = "0.39.4" @@ -5942,7 +5934,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cdcc8dd4e2f670d309a5f0e83fe36dfdc05af317008fea29144da1a2ac858e5e" dependencies = [ "memchr", - "serde", ] [[package]] @@ -5999,7 +5990,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6410,7 +6401,7 @@ dependencies = [ "errno 0.3.14", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6482,7 +6473,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs 0.26.11", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6583,9 +6574,9 @@ dependencies = [ [[package]] name = "scc" -version = "3.7.1" +version = "3.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5bcd12b6caff5213cc3c03123cde8c3db5e413008a63b0c0ba35e6275825ea92" +checksum = "7800bc2c7e91b26aeae70c2ab6eaa653efc133104b7756e674c99304adff3fdb" dependencies = [ "saa", "sdd", @@ -6847,7 +6838,7 @@ version = "1.4.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b" dependencies = [ - "errno 0.3.14", + "errno 0.2.8", "libc", ] @@ -8226,12 +8217,12 @@ dependencies = [ [[package]] name = "wayland-scanner" -version = "0.31.8" +version = "0.31.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5423e94b6a63e68e439803a3e153a9252d5ead12fd853334e2ad33997e3889e3" +checksum = "9c324a910fd86ebdc364a3e61ec1f11737d3b1d6c273c0239ee8ff4bc0d24b4a" dependencies = [ "proc-macro2", - "quick-xml 0.38.4", + "quick-xml", "quote", ] @@ -8543,7 +8534,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] @@ -8671,15 +8662,6 @@ dependencies = [ "windows-targets 0.52.6", ] -[[package]] -name = "windows-sys" -version = "0.59.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" -dependencies = [ - "windows-targets 0.52.6", -] - [[package]] name = "windows-sys" version = "0.61.2" @@ -9157,9 +9139,9 @@ dependencies = [ [[package]] name = "zbus_names" -version = "4.3.2" +version = "4.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7074f3e50b894eac91750142016d30d0a89be8e67dbfd9704fb875825760e52d" +checksum = "1039ca249fee9559680f3a9f05b55e0761fee51af4f6c1e7d8c1f31e549721d2" dependencies = [ "serde", "winnow", @@ -9168,12 +9150,12 @@ dependencies = [ [[package]] name = "zbus_xml" -version = "5.1.1" +version = "5.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8067892e940ed1727dea64690378601603b31d62dfde019a5335fbb7c0e0ed9" +checksum = "59ab0513c0a66a60a8718d5ad712a5094845a20d95b5c1c81e2bd8e904a52b4f" dependencies = [ - "quick-xml 0.39.4", "serde", + "winnow", "zbus_names", "zvariant", ] @@ -9354,9 +9336,9 @@ dependencies = [ [[package]] name = "zvariant" -version = "5.12.0" +version = "5.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a192a0bde63360d77a7523c833d4b4ce6070a927e2c53246e4c540b1a3e27be0" +checksum = "7cf057bb00bf5c9ad77abb6147b0ca4818236a1858416e9d988e40d6322fefa7" dependencies = [ "endi", "enumflags2", @@ -9368,9 +9350,9 @@ dependencies = [ [[package]] name = "zvariant_derive" -version = "5.12.0" +version = "5.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "90bc6cde9c01c511074be97f7ccb6c19d0da89e3f8662e812e999dcfd4638737" +checksum = "8118ca6bda77bfc0ab51d660db0c955f2505eef854c9a449435bccb616933b31" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -9381,9 +9363,9 @@ dependencies = [ [[package]] name = "zvariant_utils" -version = "3.4.0" +version = "3.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e8535915cfa75547e559d8c68e8139909a4aeee076831e4ef7fc59d8172c4d6" +checksum = "90cb9383f9b45290407a1258b202d3f8f01db719eb60b4e4055c6375af4fc7c7" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 333c0be..bcfdafa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,6 +68,7 @@ shlex = "1.3.0" smallvec = "1.13.2" sneed = "0.0.22" strum = { version = "0.28.0", default-features = false } +temp-dir = "0.2.0" thiserror = "2.0.16" tiny-bip39 = "2.0.0" tokio = { version = "1.50.0", default-features = false } diff --git a/app/rpc_server.rs b/app/rpc_server.rs index 294d52f..2b0435b 100644 --- a/app/rpc_server.rs +++ b/app/rpc_server.rs @@ -230,12 +230,8 @@ impl RpcServer for RpcServerImpl { } None => None, }; - let fee_sats = filled_tx - .transaction - .fee() - .map_err(custom_err)? - .unwrap() - .to_sat(); + let fee_sats = + filled_tx.transaction.fee().map_err(custom_err)?.to_sat(); let res = TxInfo { confirmations, fee_sats, diff --git a/deny.toml b/deny.toml index cd09dd2..64535ea 100644 --- a/deny.toml +++ b/deny.toml @@ -242,7 +242,6 @@ skip-tree = [ { crate = "heck@0.4.1" }, { crate = "itertools@0.11.0" }, { crate = "litrs@0.2.3" }, - { crate = "quick-xml@0.30.0" }, { crate = "redox_syscall@0.4.1" }, { crate = "rustc-hash@1.1.0" }, { crate = "rustls-platform-verifier@0.5.3" }, diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 40e7c48..61bb22b 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -67,6 +67,9 @@ zeromq = { workspace = true, optional = true } workspace = true features = ["AES256-GCM", "ECIES-MAC", "HMAC-SHA256", "x25519"] +[dev-dependencies] +temp-dir = { workspace = true } + [features] clap = ["dep:clap", "plain_bitnames_types/clap"] zmq = ["dep:zeromq"] diff --git a/lib/archive.rs b/lib/archive.rs index 133006c..04e008d 100644 --- a/lib/archive.rs +++ b/lib/archive.rs @@ -19,12 +19,15 @@ use crate::types::{ }; #[allow(clippy::duplicated_attributes)] -#[derive(thiserror::Error, transitive::Transitive, Debug)] -#[transitive(from(db::error::Put, DbError))] -#[transitive(from(db::error::TryGet, DbError))] -#[transitive(from(env::error::CreateDb, EnvError))] -#[transitive(from(env::error::WriteTxn, EnvError))] -#[transitive(from(rwtxn::error::Commit, RwTxnError))] +#[derive(Debug, thiserror::Error, transitive::Transitive)] +#[transitive( + from(db::error::Delete, DbError), + from(db::error::Put, DbError), + from(db::error::TryGet, DbError), + from(env::error::CreateDb, EnvError), + from(env::error::WriteTxn, EnvError), + from(rwtxn::error::Commit, RwTxnError) +)] pub enum Error { #[error(transparent)] Db(Box), @@ -702,6 +705,32 @@ impl Archive { }) } + /// Delete a stored body, reversing the effects of [`Self::put_body`]. + /// + /// Used to discard a body that was stored before its contents could be + /// validated (e.g. a body received from a peer) and later turned out to be + /// invalid, so that the block is reported missing again by + /// [`Self::iter_missing_bodies`] and the real body is re-requested. + pub fn delete_body( + &self, + rwtxn: &mut RwTxn, + block_hash: BlockHash, + body: &Body, + ) -> Result<(), Error> { + self.bodies.delete(rwtxn, &block_hash)?; + body.transactions.iter().try_for_each(|tx| { + let txid = tx.txid(); + let mut inclusions = self.get_tx_inclusions(rwtxn, txid)?; + inclusions.remove(&block_hash); + if inclusions.is_empty() { + self.txid_to_inclusions.delete(rwtxn, &txid)?; + } else { + self.txid_to_inclusions.put(rwtxn, &txid, &inclusions)?; + } + Ok(()) + }) + } + /// Store a header. /// /// The following predicates MUST be met before calling this function: diff --git a/lib/net/error.rs b/lib/net/error.rs index 466e2cb..4f55a32 100644 --- a/lib/net/error.rs +++ b/lib/net/error.rs @@ -41,6 +41,35 @@ pub enum AcceptConnection { ServerEndpointClosed, } +pub(in crate::net) mod configure_client { + use thiserror::Error; + + #[derive(Debug, Error)] + pub(in crate::net) enum Inner { + #[error(transparent)] + NoInitialCipherSuite( + #[from] quinn::crypto::rustls::NoInitialCipherSuite, + ), + #[error("rustls error")] + Rustls(#[source] rustls::Error), + } + + #[derive(Debug, Error)] + #[error("failed to configure p2p client")] + #[repr(transparent)] + pub struct Error(#[source] Inner); + + impl From for Error + where + Inner: From, + { + fn from(err: E) -> Self { + Self(err.into()) + } + } +} +pub use configure_client::Error as ConfigureClient; + #[allow(clippy::duplicated_attributes)] #[derive(Debug, Error, Transitive)] #[transitive(from(db::error::Put, db::Error))] @@ -58,6 +87,8 @@ pub enum Error { AlreadyConnected(#[from] AlreadyConnected), #[error("bincode error")] Bincode(#[from] bincode::Error), + #[error(transparent)] + ConfigureClient(#[from] ConfigureClient), #[error("connect error")] Connect(#[from] quinn::ConnectError), #[error(transparent)] @@ -74,8 +105,6 @@ pub enum Error { /// `0.0.0.0` is one example of an "unspecified" IP. #[error("unspecified peer ip address (cannot connect to '{0}')")] UnspecfiedPeerIP(IpAddr), - #[error(transparent)] - NoInitialCipherSuite(#[from] quinn::crypto::rustls::NoInitialCipherSuite), #[error("peer connection")] PeerConnection(#[source] Box), #[error("quinn rustls error")] diff --git a/lib/net/mod.rs b/lib/net/mod.rs index 12f7153..22ce374 100644 --- a/lib/net/mod.rs +++ b/lib/net/mod.rs @@ -91,9 +91,12 @@ impl rustls::client::danger::ServerCertVerifier for SkipServerVerification { .supported_schemes() } } -fn configure_client() --> Result { - let crypto = rustls::ClientConfig::builder() + +fn configure_client() -> Result { + let crypto_provider = Arc::new(rustls::crypto::ring::default_provider()); + let crypto = rustls::ClientConfig::builder_with_provider(crypto_provider) + .with_safe_default_protocol_versions() + .map_err(error::configure_client::Inner::Rustls)? .dangerous() .with_custom_certificate_verifier(SkipServerVerification::new()) .with_no_client_auth(); diff --git a/lib/net/peer/error.rs b/lib/net/peer/error.rs index c966d65..b687011 100644 --- a/lib/net/peer/error.rs +++ b/lib/net/peer/error.rs @@ -81,6 +81,8 @@ pub(in crate::net::peer) mod connection { ReadMagic(#[source] quinn::ReadExactError), #[error("read to end error")] ReadToEnd(#[from] quinn::ReadToEndError), + #[error("timed out waiting for response")] + Timeout, } #[derive(Debug, Error)] diff --git a/lib/net/peer/mod.rs b/lib/net/peer/mod.rs index 9795658..8ca4408 100644 --- a/lib/net/peer/mod.rs +++ b/lib/net/peer/mod.rs @@ -154,10 +154,34 @@ impl Connection { pub const HEARTBEAT_TIMEOUT_INTERVAL: Duration = Duration::from_secs(5); + pub const MIN_READ_RESPONSE_TIMEOUT: Duration = Duration::from_secs(5); + pub fn addr(&self) -> SocketAddr { self.inner.remote_address() } + /// Timeout for reading a full response from a peer, scaled to the maximum + /// permitted response size. + /// Bounds the wait for an unresponsive peer while leaving ample time for a + /// large but steadily-progressing response. + const fn response_read_timeout( + read_response_limit: NonZeroUsize, + ) -> Duration { + // Minimum sustained throughput, in bytes per second, that a peer + // streaming a response body is expected to achieve. + // Used to scale the response read timeout to `read_response_limit`, so + // a large (up to 10MB) block response is given proportionally more + // time. Deliberately conservative so a slow or congested link is not + // aborted; a peer that stops making progress entirely still hits the + // timeout. + const MIN_READ_RESPONSE_THROUGHPUT: u64 = 64 * 1024; + + let body_allowance = Duration::from_secs( + read_response_limit.get() as u64 / MIN_READ_RESPONSE_THROUGHPUT, + ); + Self::MIN_READ_RESPONSE_TIMEOUT.saturating_add(body_allowance) + } + pub fn new(connection: quinn::Connection, network: Network) -> Self { Self { inner: connection, @@ -274,10 +298,20 @@ impl Connection { } })?; send.finish()?; - Ok( - Self::receive_response(self.network, recv, read_response_limit) - .await, + // Bound the wait for a response so an unresponsive peer that holds the + // bi-stream open cannot pin the outbound request channel indefinitely. + // The timeout scales with the maximum response size, so a large (up to + // 10MB) block response on a slow or congested link is not aborted. + let response = match tokio::time::timeout( + Self::response_read_timeout(read_response_limit), + Self::receive_response(self.network, recv, read_response_limit), ) + .await + { + Ok(response) => response, + Err(_elapsed) => Err(error::connection::Receive::Timeout.into()), + }; + Ok(response) } // Send a pre-serialized response, where the response does not include @@ -512,3 +546,44 @@ pub struct Peer { pub address: SocketAddr, pub status: PeerConnectionStatus, } + +#[cfg(test)] +mod test { + use std::num::NonZeroUsize; + + use crate::{ + net::peer::{Connection, message::GetBlockRequest}, + types::BlockHash, + }; + + /// A large (up to 10MB) block response must be granted substantially more + /// time than the heartbeat timeout, so that a slow but steadily-progressing + /// response over a congested link is not spuriously aborted (which would + /// stall IBD). + #[test] + fn large_response_read_timeout_leaves_headroom() { + let get_block = GetBlockRequest { + block_hash: BlockHash([0u8; 32]), + descendant_tip: None, + ancestor: None, + peer_state_id: None, + }; + let timeout = + Connection::response_read_timeout(get_block.read_response_limit()); + assert!( + timeout > Connection::HEARTBEAT_TIMEOUT_INTERVAL, + "10MB block response timeout {timeout:?} must exceed the heartbeat \ + timeout {:?}", + Connection::HEARTBEAT_TIMEOUT_INTERVAL, + ); + } + + /// A tiny response is still bounded, but never below the base allowance + /// covering round-trip latency. + #[test] + fn small_response_read_timeout_at_least_base() { + let limit = NonZeroUsize::new(256).unwrap(); + let timeout = Connection::response_read_timeout(limit); + assert_eq!(timeout, Connection::MIN_READ_RESPONSE_TIMEOUT); + } +} diff --git a/lib/node/net_task.rs b/lib/node/net_task.rs index 2573e27..a5dbd75 100644 --- a/lib/node/net_task.rs +++ b/lib/node/net_task.rs @@ -19,7 +19,11 @@ use futures::{ stream, }; use nonempty::NonEmpty; -use sneed::{DbError, EnvError, RwTxn, RwTxnError, db}; +use sneed::{ + DbError, EnvError, RwTxn, RwTxnError, db, env::error as env_error, + rwtxn::error as rwtxn_error, +}; +use thiserror::Error; use tokio::task::{self, JoinHandle}; use tokio_stream::StreamNotifyClose; @@ -37,13 +41,17 @@ use crate::{ BmmResult, Body, Header, Tip, proto::{self, mainchain}, }, - util::join_set, + util::{ErrorChain, join_set}, }; #[allow(clippy::duplicated_attributes)] -#[derive(thiserror::Error, transitive::Transitive, Debug)] -#[transitive(from(db::error::IterInit, DbError))] -#[transitive(from(db::error::IterItem, DbError))] +#[derive(transitive::Transitive, Debug, Error)] +#[transitive( + from(db::error::IterInit, DbError), + from(db::error::IterItem, DbError), + from(env_error::WriteTxn, EnvError), + from(rwtxn_error::Commit, RwTxnError) +)] pub enum Error { #[error("archive error")] Archive(#[from] archive::Error), @@ -260,6 +268,11 @@ fn disconnect_tip_( /// The new tip block and all ancestor blocks must exist in the node's archive. /// A result of `Ok(true)` indicates a successful re-org. /// A result of `Ok(false)` indicates that no re-org was attempted. +// a state error means a peer sent an invalid block; it must not be fatal +fn is_fatal_reorg_error(err: &Error) -> bool { + !matches!(err, Error::State(_)) +} + fn reorg_to_tip( env: &sneed::Env, archive: &Archive, @@ -390,7 +403,7 @@ fn reorg_to_tip( } two_way_peg_data }; - let () = connect_tip_( + let () = match connect_tip_( &mut rwtxn, archive, mempool, @@ -398,7 +411,24 @@ fn reorg_to_tip( header, body, &two_way_peg_data, - )?; + ) { + Ok(()) => (), + Err(err) => { + if is_fatal_reorg_error(&err) { + // The stored body for this block failed validation (e.g. a peer + // supplied a body whose contents do not match the header's merkle + // root). Abort the reorg and discard the invalid body from the + // archive so that the block is reported missing again and the real + // body is re-requested, instead of the archive staying poisoned. + drop(rwtxn); + let mut rwtxn = env.write_txn()?; + let () = + archive.delete_body(&mut rwtxn, header.hash(), body)?; + rwtxn.commit()?; + } + return Err(err); + } + }; let new_tip_hash = state.try_get_tip(&rwtxn)?.unwrap(); let bmm_verification = archive.get_best_main_verification(&rwtxn, new_tip_hash)?; @@ -1022,8 +1052,8 @@ impl NetTask { } } } - MailboxItem::NewTipReady(new_tip, _addr, resp_tx) => { - let reorg_applied = task::block_in_place(|| { + MailboxItem::NewTipReady(new_tip, addr, resp_tx) => { + let reorg_result = task::block_in_place(|| { reorg_to_tip( &self.ctxt.env, &self.ctxt.archive, @@ -1033,7 +1063,27 @@ impl NetTask { &self.ctxt.zmq_pub_handler, new_tip, ) - })?; + }); + let reorg_applied = match reorg_result { + Ok(applied) => applied, + Err(err) if is_fatal_reorg_error(&err) => { + return Err(err); + } + // an invalid block must not kill the net task; drop the + // peer and keep running + Err(err) => { + tracing::warn!( + ?new_tip, + ?addr, + err = format!("{:#}", ErrorChain::new(&err)), + "rejecting invalid tip from peer" + ); + if let Some(addr) = addr { + let () = self.ctxt.net.remove_active_peer(addr); + } + false + } + }; if let Some(resp_tx) = resp_tx { let () = resp_tx .send(reorg_applied) @@ -1259,3 +1309,24 @@ impl Drop for NetTaskHandle { } } } + +#[cfg(test)] +mod test { + use crate::{ + node::net_task::{Error, is_fatal_reorg_error}, + state, + }; + + // a peer's invalid block (value out > value in) must not be fatal + #[test] + fn invalid_peer_block_is_not_fatal() { + let err = Error::State(Box::new(state::Error::NotEnoughFees)); + assert!(!is_fatal_reorg_error(&err)); + } + + // local infrastructure errors stay fatal + #[test] + fn infrastructure_error_is_fatal() { + assert!(is_fatal_reorg_error(&Error::PeerInfoRxClosed)); + } +} diff --git a/lib/state/block.rs b/lib/state/block.rs index 1f0e999..266fed5 100644 --- a/lib/state/block.rs +++ b/lib/state/block.rs @@ -693,7 +693,8 @@ mod test { #[test] fn disconnect_bitname_data_update() -> anyhow::Result<()> { - let (env, state) = fresh_state("disconnect_bitname_data_update")?; + let (_temp_dir, env, state) = + fresh_state("disconnect_bitname_data_update")?; let signing_key = SigningKey::from_bytes(&[7; 32]); let verifying_key = signing_key.verifying_key().into(); let address = authorization::get_address(&verifying_key); diff --git a/lib/state/error.rs b/lib/state/error.rs index 5d9213e..8c57944 100644 --- a/lib/state/error.rs +++ b/lib/state/error.rs @@ -6,7 +6,7 @@ use transitive::Transitive; use crate::types::{ AmountOverflowError, AmountUnderflowError, BitName as BitNameId, BlockHash, - M6id, MerkleRoot, OutPoint, Txid, WithdrawalBundleError, + M6id, MerkleRoot, OutPoint, Txid, WithdrawalBundleError, transaction, }; /// Errors related to BitNames @@ -187,6 +187,8 @@ pub enum Error { #[error(transparent)] BorshSerialize(borsh::io::Error), #[error(transparent)] + ComputeFee(#[from] transaction::ComputeFeeError), + #[error(transparent)] ComputeMerkleRoot(#[from] crate::types::ComputeMerkleRootError), #[error(transparent)] ConnectWithdrawalBundleSubmitted(#[from] ConnectWithdrawalBundleSubmitted), @@ -213,14 +215,14 @@ pub enum Error { NoDepositBlock, #[error("total fees less than coinbase value")] NotEnoughFees, - #[error("value in is less than value out")] - NotEnoughValueIn, #[error("stxo {outpoint} doesn't exist")] NoStxo { outpoint: OutPoint }, #[error("no tip")] NoTip, #[error(transparent)] NoUtxo(#[from] NoUtxo), + #[error("withdrawal output {outpoint} cannot be spent by a transaction")] + SpendWithdrawalOutput { outpoint: OutPoint }, #[error("Withdrawal bundle event block doesn't exist")] NoWithdrawalBundleEventBlock, #[error(transparent)] diff --git a/lib/state/mod.rs b/lib/state/mod.rs index 24b0376..2b55e09 100644 --- a/lib/state/mod.rs +++ b/lib/state/mod.rs @@ -497,7 +497,17 @@ impl State { let () = self.validate_reservations(tx)?; let () = self.validate_bitnames(rotxn, tx)?; let () = self.validate_batch_icann(tx)?; - tx.fee()?.ok_or(Error::NotEnoughValueIn) + for (outpoint, output) in tx.spent_inputs() { + // a withdrawal output is committed to a bundle and can only be + // spent by the bundle, never by a transaction + if output.content.is_withdrawal() { + return Err(Error::SpendWithdrawalOutput { + outpoint: *outpoint, + }); + } + } + let fee = tx.fee()?; + Ok(fee) } pub fn validate_transaction( @@ -687,6 +697,7 @@ impl Watchable<()> for State { #[cfg(test)] mod test { + use bitcoin::hashes::Hash as _; use ed25519_dalek::SigningKey; use crate::{ @@ -697,37 +708,38 @@ mod test { FilledOutputContent, FilledTransaction, Hash, InPoint, MutableBitNameData, OutPoint, OutPointKey, Output, OutputContent, SpentOutput, Transaction, TxData, Txid, VerifyingKey, + WithdrawalOutputContent, }, }; - pub fn temp_env_path( - test_name: &str, - ) -> anyhow::Result { - let mut path = std::env::temp_dir(); + fn temp_dir(test_name: &str) -> anyhow::Result { let nanos = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH)? .as_nanos(); - path.push(format!( + let res = temp_dir::TempDir::with_prefix(format!( "bitnames-{test_name}-{}-{nanos}", std::process::id() - )); - Ok(path) + ))?; + Ok(res) } // open a fresh state-backed env in a unique temp dir - pub fn temp_env(test_name: &str) -> anyhow::Result { - let path = temp_env_path(test_name)?; - std::fs::create_dir_all(&path)?; + pub fn temp_env( + test_name: &str, + ) -> anyhow::Result<(temp_dir::TempDir, sneed::Env)> { + let temp_dir = temp_dir(test_name)?; let mut opts = heed::EnvOpenOptions::new(); opts.map_size(64 * 1024 * 1024).max_dbs(State::NUM_DBS); - let res = unsafe { sneed::Env::open(&opts, &path) }?; - Ok(res) + let env = unsafe { sneed::Env::open(&opts, temp_dir.path()) }?; + Ok((temp_dir, env)) } - pub fn fresh_state(test_name: &str) -> anyhow::Result<(sneed::Env, State)> { - let env = temp_env(test_name)?; + pub fn fresh_state( + test_name: &str, + ) -> anyhow::Result<(temp_dir::TempDir, sneed::Env, State)> { + let (temp_dir, env) = temp_env(test_name)?; let state = State::new(&env)?; - Ok((env, state)) + Ok((temp_dir, env, state)) } /// Create a bitcoin filled output @@ -801,7 +813,7 @@ mod test { #[test] fn validate_transaction_rejects_missing_authorization() -> anyhow::Result<()> { - let (env, state) = fresh_state("auth_count")?; + let (_temp_dir, env, state) = fresh_state("auth_count")?; let signing_key = SigningKey::from_bytes(&[1u8; 32]); let verifying_key: VerifyingKey = signing_key.verifying_key().into(); let address = authorization::get_address(&verifying_key); @@ -848,7 +860,7 @@ mod test { #[test] fn validate_bitnames_rejects_registration_without_matching_reservation() -> anyhow::Result<()> { - let (env, state) = fresh_state("registration")?; + let (_temp_dir, env, state) = fresh_state("registration")?; let rotxn = env.read_txn()?; let name_hash = BitName([7; 32]); let revealed_nonce: Hash = [3; 32]; @@ -881,13 +893,55 @@ mod test { Ok(()) } + #[test] + fn cannot_spend_withdrawal_output() -> anyhow::Result<()> { + let (_temp_dir, env, state) = + fresh_state("cannot-spend-withdrawal-output")?; + let main_address = { + let pkh = bitcoin::PubkeyHash::hash(b"test pubkey"); + bitcoin::Address::p2pkh(pkh, bitcoin::NetworkKind::Test) + .into_unchecked() + }; + let withdrawal = FilledOutput { + address: Address::ALL_ZEROS, + content: FilledOutputContent::BitcoinWithdrawal( + WithdrawalOutputContent { + value: bitcoin::Amount::from_sat(1000), + main_fee: bitcoin::Amount::from_sat(300), + main_address, + }, + ), + memo: Vec::new(), + }; + let outpoint = OutPoint::Regular { + txid: [1; 32].into(), + vout: 0, + }; + let tx = FilledTransaction { + transaction: Transaction { + inputs: vec![outpoint], + outputs: vec![ + bitcoin_filled_output(Address::ALL_ZEROS, 1300).into(), + ], + ..Default::default() + }, + spent_utxos: vec![withdrawal], + }; + let rotxn = env.read_txn()?; + assert!(matches!( + state.validate_filled_transaction(&rotxn, &tx), + Err(crate::state::Error::SpendWithdrawalOutput { .. }) + )); + Ok(()) + } + #[test] fn sidechain_wealth() -> anyhow::Result<()> { use std::str::FromStr; use bitcoin::hashes::Hash as _; - let (env, state) = fresh_state("sidechain-wealth")?; + let (_temp_dir, env, state) = fresh_state("sidechain-wealth")?; { let mut rwtxn = env.write_txn()?; diff --git a/lib/state/two_way_peg_data.rs b/lib/state/two_way_peg_data.rs index 7afd45d..e666c5b 100644 --- a/lib/state/two_way_peg_data.rs +++ b/lib/state/two_way_peg_data.rs @@ -878,7 +878,11 @@ fn disconnect_event( if !state.utxos.delete(rwtxn, &OutPointKey::from(&outpoint))? { return Err(error::NoUtxo { outpoint }.into()); } - *latest_deposit_block_hash = Some(event_block_hash); + // Blocks are iterated in reverse here, so the first event block + // hash seen is the latest. Keep it to match what `connect` stored. + if latest_deposit_block_hash.is_none() { + *latest_deposit_block_hash = Some(event_block_hash); + } } BlockEvent::WithdrawalBundle(withdrawal_bundle_event) => { let () = disconnect_withdrawal_bundle_event( @@ -887,7 +891,12 @@ fn disconnect_event( block_height, withdrawal_bundle_event, )?; - *latest_withdrawal_bundle_event_block_hash = Some(event_block_hash); + // Blocks are iterated in reverse here, so the first event block + // hash seen is the latest. Keep it to match what `connect` stored. + if latest_withdrawal_bundle_event_block_hash.is_none() { + *latest_withdrawal_bundle_event_block_hash = + Some(event_block_hash); + } } } Ok(()) @@ -995,7 +1004,7 @@ pub fn disconnect( #[cfg(test)] mod test { - use std::{collections::BTreeMap, sync::Arc}; + use std::collections::BTreeMap; use bitcoin::{ Network, @@ -1009,7 +1018,7 @@ mod test { HeightStamped, RollBack, State, WithdrawalBundleInfo, test::{bitcoin_filled_output, fresh_state}, two_way_peg_data::{ - collect_withdrawal_bundle, disconnect, + collect_withdrawal_bundle, connect, disconnect, disconnect_withdrawal_bundle_failed, }, }, @@ -1018,7 +1027,7 @@ mod test { OutPoint, OutPointKey, Txid, WithdrawalBundle, WithdrawalBundleEvent, WithdrawalBundleEventStatus, WithdrawalBundleStatus, WithdrawalOutputContent, - proto::mainchain::{BlockEvent, BlockInfo, TwoWayPegData}, + proto::mainchain::{BlockEvent, BlockInfo, Deposit, TwoWayPegData}, }, }; @@ -1026,7 +1035,7 @@ mod test { // the failure must spend them again #[test] fn disconnect_failed_bundle_spends_reinstated_utxo() -> anyhow::Result<()> { - let (env, state) = + let (_temp_dir, env, state) = fresh_state("disconnect_failed_bundle_spends_reinstated_utxo")?; let outpoint = OutPoint::Regular { txid: Txid::from([1; 32]), @@ -1083,7 +1092,7 @@ mod test { #[test] fn disconnect_withdrawal_event_block_uses_correct_db() -> anyhow::Result<()> { - let (env, state) = + let (_temp_dir, env, state) = fresh_state("disconnect_withdrawal_event_block_uses_correct_db")?; let block_height = 5u32; @@ -1175,7 +1184,7 @@ mod test { >, f: impl FnOnce(&State, &mut sneed::RwTxn<'_>) -> R, ) -> anyhow::Result { - let (env, state) = fresh_state(test_name)?; + let (_temp_dir, env, state) = fresh_state(test_name)?; let res = { let mut rwtxn = env.write_txn()?; state.height.put( @@ -1215,10 +1224,6 @@ mod test { } f(&state, &mut rwtxn) }; - drop(state); - let env_path = Arc::clone(env.path()); - drop(env); - drop(std::fs::remove_dir_all(env_path)); Ok(res) } @@ -1260,7 +1265,7 @@ mod test { Body, FilledTransaction, Header, proto::mainchain::Deposit, }; - let (env, state) = fresh_state("deposit_reorg_round_trips")?; + let (_temp_dir, env, state) = fresh_state("deposit_reorg_round_trips")?; let empty_body = Body { coinbase: Vec::new(), transactions: Vec::new(), @@ -1335,4 +1340,55 @@ mod test { Ok(()) } + + // A single two-way-peg batch can span multiple mainchain blocks. Connecting + // deposits from two distinct blocks then disconnecting the batch must + // restore the prior state. Before the fix, disconnect recomputed the latest + // deposit block hash by reverse iteration (yielding the oldest block) and + // panicked on the consistency assert against the newest hash connect stored. + #[test] + fn disconnect_two_deposit_blocks_restores_state() -> anyhow::Result<()> { + fn deposit_block(salt: u8) -> (bitcoin::BlockHash, BlockInfo) { + let dep = Deposit { + tx_index: 0, + outpoint: bitcoin::OutPoint { + txid: bitcoin::Txid::from_byte_array([salt; 32]), + vout: 0, + }, + output: FilledOutput { + address: Address([salt; 20]), + content: FilledOutputContent::new_bitcoin_value( + bitcoin::Amount::from_sat(1000), + ), + memo: Vec::new(), + }, + }; + ( + bitcoin::BlockHash::from_byte_array([salt; 32]), + BlockInfo { + bmm_commitment: None, + events: vec![BlockEvent::Deposit(dep)], + }, + ) + } + let (_temp_dir, env, state) = + fresh_state("disconnect_two_deposit_blocks_restores_state")?; + let mut rwtxn = env.write_txn()?; + state.height.put(&mut rwtxn, &(), &10)?; + + let mut block_info = LinkedHashMap::new(); + let (h1, b1) = deposit_block(1); + let (h2, b2) = deposit_block(2); + block_info.insert(h1, b1); + block_info.insert(h2, b2); + let tdp = TwoWayPegData { block_info }; + + let () = connect(&state, &mut rwtxn, &tdp)?; + anyhow::ensure!(state.utxos.len(&rwtxn)? == 2); + disconnect(&state, &mut rwtxn, &tdp)?; + + anyhow::ensure!(state.utxos.len(&rwtxn)? == 0); + anyhow::ensure!(state.deposit_blocks.len(&rwtxn)? == 0); + Ok(()) + } } diff --git a/types/lib.rs b/types/lib.rs index c9ac2dc..87a59e5 100644 --- a/types/lib.rs +++ b/types/lib.rs @@ -22,7 +22,7 @@ pub mod constants; pub mod hashes; pub mod keys; pub mod schema; -mod transaction; +pub mod transaction; pub use address::Address; pub use bitname_data::{ diff --git a/types/transaction/mod.rs b/types/transaction/mod.rs index 249d15f..90aeb8d 100644 --- a/types/transaction/mod.rs +++ b/types/transaction/mod.rs @@ -3,6 +3,7 @@ use std::{borrow::Borrow, io::Cursor}; use bitcoin::amount::CheckedSum; use borsh::{self, BorshDeserialize, BorshSerialize}; use serde::{Deserialize, Serialize}; +use thiserror::Error; use utoipa::{PartialSchema, ToSchema}; #[cfg(feature = "heed")] @@ -662,6 +663,16 @@ pub struct SpentOutput { pub inpoint: InPoint, } +#[derive(Debug, Error)] +pub enum ComputeFeeError { + #[error("underfunded (value in < value out)")] + Underfunded, + #[error("value in overflow")] + ValueInOverflow(#[source] AmountOverflowError), + #[error("value out overflow")] + ValueOutOverflow(#[source] AmountOverflowError), +} + #[derive(Clone, Debug, Deserialize, Serialize)] pub struct FilledTransaction { pub transaction: Transaction, @@ -802,13 +813,17 @@ impl FilledTransaction { /// returns the difference between the value spent and value out, if it is /// non-negative. - pub fn fee(&self) -> Result, AmountOverflowError> { - let spent_value = self.spent_value()?; - let value_out = self.value_out()?; + pub fn fee(&self) -> Result { + let spent_value = self + .spent_value() + .map_err(ComputeFeeError::ValueInOverflow)?; + let value_out = self + .value_out() + .map_err(ComputeFeeError::ValueOutOverflow)?; if spent_value < value_out { - Ok(None) + Err(ComputeFeeError::Underfunded) } else { - Ok(Some(spent_value - value_out)) + Ok(spent_value - value_out) } }