diff --git a/.github/workflows/check_lint_build.yaml b/.github/workflows/check_lint_build.yaml index 3b92b49..43b1659 100644 --- a/.github/workflows/check_lint_build.yaml +++ b/.github/workflows/check_lint_build.yaml @@ -7,11 +7,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: @@ -28,27 +42,83 @@ 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: release-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: - shared-key: x86_64-unknown-linux-gnu + cache-all-crates: "true" + lookup-only: "true" + prefix-key: "v0-rust-nightly" + shared-key: release-x86_64-unknown-linux-gnu - name: Rustfmt run: cargo fmt --all -- --check - name: Cargo check - run: cargo check --all-targets --all-features + run: cargo check --release --all-targets --all-features - name: Clippy - run: cargo clippy --all-targets --all-features + run: cargo clippy --release --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: | @@ -82,41 +152,31 @@ jobs: 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: - 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: - shared-key: x86_64-unknown-linux-gnu + cache-all-crates: "true" + prefix-key: "v0-rust-nightly" + shared-key: release-x86_64-unknown-linux-gnu - name: Build (release) - run: cargo build --release + run: cargo build --release --bin photon_app --example integration_tests - name: Run integration tests id: runIntegrationTests @@ -125,9 +185,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 PHOTON_APP='target/release/photon_app' - cargo run --release --example integration_tests + target/release/examples/integration_tests test-build-release: strategy: @@ -156,6 +216,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 @@ -169,11 +233,9 @@ jobs: - name: Rust Cache uses: Swatinem/rust-cache@v2.9.1 with: - 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 @@ -294,4 +356,4 @@ jobs: password: ${{ secrets.RELEASES_SERVER_PW }} port: 22 source: 'L2-S99-Photon-latest-*.zip' - target: '/var/www/html/' \ No newline at end of file + target: '/var/www/html/' 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 b878ca7..8474a99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -294,9 +294,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.102" +version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3" [[package]] name = "arboard" @@ -314,7 +314,7 @@ dependencies = [ "objc2-foundation 0.3.2", "parking_lot", "percent-encoding", - "windows-sys 0.59.0", + "windows-sys 0.52.0", "x11rb", ] @@ -1603,9 +1603,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", ] @@ -5284,6 +5284,7 @@ dependencies = [ "smallvec", "sneed 0.0.11", "strum 0.27.2", + "temp-dir", "thiserror 2.0.18", "tiny-bip39", "tokio", @@ -5781,15 +5782,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" @@ -5797,7 +5789,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cdcc8dd4e2f670d309a5f0e83fe36dfdc05af317008fea29144da1a2ac858e5e" dependencies = [ "memchr", - "serde", ] [[package]] @@ -5854,7 +5845,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6285,7 +6276,7 @@ dependencies = [ "errno 0.3.14", "libc", "linux-raw-sys 0.4.15", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6357,7 +6348,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs 0.26.11", - "windows-sys 0.59.0", + "windows-sys 0.52.0", ] [[package]] @@ -6461,9 +6452,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", @@ -8149,12 +8140,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", ] @@ -8603,15 +8594,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" @@ -9134,9 +9116,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", @@ -9145,12 +9127,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", ] @@ -9331,9 +9313,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", @@ -9345,9 +9327,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", @@ -9358,9 +9340,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 060f2dd..4f9d5cc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,7 @@ shlex = "1.3.0" smallvec = "1.13.2" sneed = "0.0.11" strum = { version = "0.27.2", features = ["derive"] } +temp-dir = "0.2.0" thiserror = "2.0.11" tiny-bip39 = "2.0.0" tokio = { version = "1.50.0", default-features = false } diff --git a/app/app.rs b/app/app.rs index 05f82d6..334437d 100644 --- a/app/app.rs +++ b/app/app.rs @@ -449,19 +449,20 @@ impl App { } else { let coinbase = Vec::new(); let (merkle_root, roots) = { - let mut accumulator = if let Some(tip_hash) = tip_hash { - let rotxn = self - .node - .env() - .read_txn() - .map_err(node::Error::from)?; - self.node - .archive() - .get_accumulator(&rotxn, tip_hash) - .map_err(node::Error::from)? - } else { - types::Accumulator::default() - }; + let mut accumulator = + if let Some(prev_side_hash) = prev_side_hash { + let rotxn = self + .node + .env() + .read_txn() + .map_err(node::Error::from)?; + self.node + .archive() + .get_accumulator(&rotxn, prev_side_hash) + .map_err(node::Error::from)? + } else { + types::Accumulator::default() + }; let merkle_root = photon::types::Body::modify_memforest::< FilledTransaction, >( diff --git a/deny.toml b/deny.toml index ab9ed34..4bc339f 100644 --- a/deny.toml +++ b/deny.toml @@ -74,11 +74,13 @@ ignore = [ #{ id = "RUSTSEC-0000-0000", reason = "you can specify a reason the advisory is ignored" }, #"a-crate-that-is-yanked@0.1.1", # you can also ignore yanked crate versions if you wish #{ crate = "a-crate-that-is-yanked@0.1.1", reason = "you can specify why you are ignoring the yanked crate" }, - { id = "RUSTSEC-2024-0370", reason = "Temporary ignore until the ouroboros branch gets updated"}, + { id = "RUSTSEC-2024-0370", reason = "Temporary ignore until the ouroboros branch gets updated" }, # paste (via aws-lc-rs) is unmaintained # https://github.com/aws/aws-lc-rs/issues/722 - { id = "RUSTSEC-2024-0436", reason = "Temporary ignore until aws-lc-rs and rustls-webpki get updated"}, - { id = "RUSTSEC-2025-0141", reason = "Temporary ignore until heed migrates to wincode"}, + { id = "RUSTSEC-2024-0436", reason = "Temporary ignore until aws-lc-rs and rustls-webpki get updated" }, + { id = "RUSTSEC-2025-0141", reason = "Temporary ignore until heed migrates to wincode" }, + # ttf-parser (via egui) is unmaintained + { id = "RUSTSEC-2026-0192", reason = "Temporary ignore until ttf-parser is replaced by skrifa etc." }, ] # If this is true, then cargo deny will use the git executable to fetch advisory database. # If this is false, then it uses a built-in git library. @@ -241,7 +243,6 @@ skip-tree = [ { crate = "hex-conservative@0.1.2" }, { crate = "itertools@0.11.0" }, { crate = "litrs@0.2.3" }, - { crate = "quick-xml@0.30.0" }, { crate = "redox_syscall@0.4.1" }, { crate = "regex-automata@0.1.10" }, { crate = "regex-syntax@0.6.29" }, @@ -272,7 +273,6 @@ unknown-git = "deny" allow-registry = ["https://github.com/rust-lang/crates.io-index"] # List of URLs for allowed Git repositories allow-git = [ - "https://github.com/Ash-L2L/bip300301.git", "https://github.com/Ash-L2L/l2l-openapi.git", "https://gitlab.com/A-Manning/leonhard-llc-ops", "https://github.com/erikjohnston/ouroboros.git?rev=ea82b33947f60c511dac6eb3815b6af3f6c3f555", diff --git a/lib/Cargo.toml b/lib/Cargo.toml index e2bdbaf..0dd8f67 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -65,6 +65,7 @@ utoipa = { workspace = true, features = ["macros", "non_strict_integers"] } [dev-dependencies] rand = { workspace = true, features = ["std_rng"] } +temp-dir = { workspace = true } [features] clap = ["dep:clap"] diff --git a/lib/archive.rs b/lib/archive.rs index 95222d0..d45118e 100644 --- a/lib/archive.rs +++ b/lib/archive.rs @@ -20,6 +20,7 @@ use crate::types::{ #[allow(clippy::duplicated_attributes)] #[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) @@ -730,6 +731,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/mempool.rs b/lib/mempool.rs index 183ed9b..d3e8463 100644 --- a/lib/mempool.rs +++ b/lib/mempool.rs @@ -153,6 +153,12 @@ impl MemPool { } /// regenerate utreexo proofs for all txs in the mempool + /// + /// A transaction whose inputs can no longer be proven against the + /// accumulator (eg. because they were spent by a just-connected block via + /// a conflicting transaction) is no longer valid. Such a transaction is + /// evicted from the mempool, along with its descendants, rather than + /// propagating an error that would abort block connect/disconnect. pub fn regenerate_proofs( &self, rwtxn: &mut RwTxn, @@ -165,18 +171,36 @@ impl MemPool { .collect() .map_err(DbError::from)?; for txid in txids { - let mut tx = - self.transactions.get(rwtxn, &txid).map_err(DbError::from)?; + // The tx may already have been evicted as a descendant of an + // earlier invalidated tx. + let Some(mut tx) = self + .transactions + .try_get(rwtxn, &txid) + .map_err(DbError::from)? + else { + continue; + }; let targets: Vec<_> = tx .transaction .inputs .iter() .map(|(_, utxo_hash)| utxo_hash.into()) .collect(); - tx.transaction.proof = accumulator.prove(&targets)?; - self.transactions - .put(rwtxn, &txid, &tx) - .map_err(DbError::from)?; + match accumulator.prove(&targets) { + Ok(proof) => { + tx.transaction.proof = proof; + self.transactions + .put(rwtxn, &txid, &tx) + .map_err(DbError::from)?; + } + Err(_) => { + tracing::debug!( + "evicting mempool transaction {txid}: inputs no \ + longer in accumulator" + ); + let () = self.delete(rwtxn, txid)?; + } + } } Ok(()) } diff --git a/lib/net/error.rs b/lib/net/error.rs index 5710f06..b92b41b 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 d110540..81cdb8b 100644 --- a/lib/net/mod.rs +++ b/lib/net/mod.rs @@ -100,9 +100,11 @@ impl rustls::client::danger::ServerCertVerifier for SkipServerVerification { } } -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 d369611..fee4ff4 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 6be0cb2..798c9a2 100644 --- a/lib/net/peer/mod.rs +++ b/lib/net/peer/mod.rs @@ -151,10 +151,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, @@ -271,10 +295,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 @@ -509,3 +543,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 21dc9e7..9f0e323 100644 --- a/lib/node/net_task.rs +++ b/lib/node/net_task.rs @@ -19,7 +19,10 @@ 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; @@ -38,13 +41,17 @@ use crate::{ BmmResult, Body, Header, MerkleRoot, Tip, proto::{self, mainchain}, }, - util::join_set, + util::{ErrorChain, join_set}, }; #[allow(clippy::duplicated_attributes)] #[derive(transitive::Transitive, Debug, Error)] -#[transitive(from(db::error::IterInit, DbError))] -#[transitive(from(db::error::IterItem, DbError))] +#[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), @@ -251,6 +258,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, @@ -381,7 +393,7 @@ fn reorg_to_tip( } two_way_peg_data }; - let () = connect_tip_( + let () = match connect_tip_( &mut rwtxn, archive, mempool, @@ -389,7 +401,27 @@ 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)?; @@ -997,8 +1029,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, @@ -1006,7 +1038,27 @@ impl NetTask { &self.ctxt.state, 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) @@ -1234,3 +1286,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(state::Error::NotEnoughValueIn); + 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 3d2ebd8..253517a 100644 --- a/lib/state/block.rs +++ b/lib/state/block.rs @@ -636,7 +636,7 @@ mod test { OutPointKey, PointedOutput, Transaction, hash, }, }; - let (env, state) = + let (_temp_dir, env, state) = fresh_state("validation_rejects_outpoint_utxo_hash_mismatch")?; // Attacker key (owns A). Victim key (owns B). diff --git a/lib/state/error.rs b/lib/state/error.rs index 0a7de8e..ba70955 100644 --- a/lib/state/error.rs +++ b/lib/state/error.rs @@ -162,6 +162,8 @@ pub enum Error { NotEnoughValueIn, #[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 5539403..25464a2 100644 --- a/lib/state/mod.rs +++ b/lib/state/mod.rs @@ -358,7 +358,14 @@ impl State { let () = Self::validate_utxo_hashes(transaction)?; let mut value_in = bitcoin::Amount::ZERO; let mut value_out = bitcoin::Amount::ZERO; - for utxo in &transaction.spent_utxos { + for (outpoint, _, utxo) in transaction.inputs() { + // a withdrawal output is committed to a bundle and can only be + // spent by the bundle, never by a transaction + if utxo.content.is_withdrawal() { + return Err(Error::SpendWithdrawalOutput { + outpoint: *outpoint, + }); + } value_in = value_in .checked_add(utxo.get_value()) .ok_or(AmountOverflowError)?; @@ -591,39 +598,44 @@ impl Watchable<()> for State { #[cfg(test)] mod test { + use bitcoin::hashes::Hash as _; + use crate::{ state::State, types::{ - Address, InPoint, OutPoint, OutPointKey, Output, OutputContent, - SpentOutput, + Address, FilledTransaction, InPoint, OutPoint, OutPointKey, Output, + OutputContent, PointedOutputRef, SpentOutput, Transaction, hash, }, }; - 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!("photon-{test_name}-{}-{nanos}", std::process::id())); - Ok(path) + let res = temp_dir::TempDir::with_prefix(format!( + "photon-{test_name}-{}-{nanos}", + std::process::id() + ))?; + 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 value output @@ -634,13 +646,53 @@ mod test { } } + #[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 = Output { + address: Address::ALL_ZEROS, + content: OutputContent::Withdrawal { + value: bitcoin::Amount::from_sat(1000), + main_fee: bitcoin::Amount::from_sat(300), + main_address, + }, + }; + let outpoint = OutPoint::Regular { + txid: [1; 32].into(), + vout: 0, + }; + let utxo_hash = hash(&PointedOutputRef { + outpoint, + output: &withdrawal, + }); + let tx = FilledTransaction { + transaction: Transaction { + inputs: vec![(outpoint, utxo_hash)], + outputs: vec![value_output(Address::ALL_ZEROS, 1300)], + ..Default::default() + }, + spent_utxos: vec![withdrawal], + }; + assert!(matches!( + state.validate_filled_transaction(&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 b514fae..5b57e54 100644 --- a/lib/state/two_way_peg_data.rs +++ b/lib/state/two_way_peg_data.rs @@ -982,7 +982,11 @@ fn disconnect_event( } let utxo_hash = hash(&PointedOutput { outpoint, output }); accumulator_diff.remove(utxo_hash.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( @@ -992,7 +996,12 @@ fn disconnect_event( accumulator_diff, 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(()) @@ -1109,7 +1118,7 @@ pub fn disconnect( #[cfg(test)] mod test { - use std::{collections::BTreeMap, sync::Arc}; + use std::collections::BTreeMap; use bitcoin::{ Network, @@ -1124,7 +1133,7 @@ mod test { rollback::RollBack, test::{fresh_state, value_output}, two_way_peg_data::{ - collect_withdrawal_bundle, disconnect, + collect_withdrawal_bundle, connect, disconnect, disconnect_withdrawal_bundle_failed, }, }, @@ -1133,7 +1142,7 @@ mod test { Output, OutputContent, Txid, WithdrawalBundle, WithdrawalBundleEvent, WithdrawalBundleEventStatus, WithdrawalBundleStatus, - proto::mainchain::{BlockEvent, BlockInfo, TwoWayPegData}, + proto::mainchain::{BlockEvent, BlockInfo, Deposit, TwoWayPegData}, }, }; @@ -1141,7 +1150,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]), @@ -1203,7 +1212,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; @@ -1292,7 +1301,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( @@ -1329,10 +1338,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) } @@ -1374,7 +1379,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(), @@ -1451,4 +1456,54 @@ 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: Output { + address: Address([salt; 20]), + content: OutputContent::Value(bitcoin::Amount::from_sat( + 1000, + )), + }, + }; + ( + 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(()) + } }