Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .github/workflows/0fc-integration.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: CI Checks - 0FC Integration Tests

on: [push, pull_request]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
build-and-test:
timeout-minutes: 60
runs-on: self-hosted
steps:
- name: Checkout source code
uses: actions/checkout@v4
- name: Install Rust stable toolchain
run: |
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
- name: Enable caching for bitcoind
id: cache-bitcoind
uses: actions/cache@v4
with:
path: bin/bitcoind-${{ runner.os }}-${{ runner.arch }}
key: bitcoind-29.0-${{ runner.os }}-${{ runner.arch }}
- name: Enable caching for electrs
id: cache-electrs
uses: actions/cache@v4
with:
path: bin/electrs-${{ runner.os }}-${{ runner.arch }}
key: electrs-submit-package-${{ runner.os }}-${{ runner.arch }}
- name: Download bitcoind
if: "steps.cache-bitcoind.outputs.cache-hit != 'true'"
run: |
source ./scripts/download_bitcoind_electrs.sh
mkdir -p bin
mv "$BITCOIND_EXE" bin/bitcoind-${{ runner.os }}-${{ runner.arch }}
- name: Download electrs
if: "steps.cache-electrs.outputs.cache-hit != 'true'"
run: |
source ./scripts/build_electrs.sh
mkdir -p bin
mv "$ELECTRS_EXE" bin/electrs-${{ runner.os }}-${{ runner.arch }}
- name: Set bitcoind/electrs environment variables
run: |
echo "BITCOIND_EXE=$( pwd )/bin/bitcoind-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV"
echo "ELECTRS_EXE=$( pwd )/bin/electrs-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV"
- name: Test with 0FC enabled
run: |
RUSTFLAGS="--cfg no_download --cfg cycle_tests --cfg tokio_unstable --cfg zero_fee_commitment_tests" cargo test -- --test-threads=1

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 great. Maybe we could also add interop test coverage as a follow-up? Presumably with Eclair it should work?

2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
- `EsploraSyncConfig` and `ElectrumSyncConfig` now support `force_wallet_full_scan`. When set,
the on-chain wallet keeps using BDK `full_scan` instead of incremental sync until a full scan
succeeds, allowing restored wallets to rediscover funds sent to previously-unknown addresses.
- `Config::anchor_channels_config` is no longer optional, hence anchor channels can no longer be
disabled. We still negotiate legacy channels if the peer does not support anchor channels.

## Bug Fixes and Improvements
- Building a fresh node against a Bitcoin Core RPC or REST chain source that fails to return the
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ check-cfg = [
"cfg(eclair_test)",
"cfg(cycle_tests)",
"cfg(hrn_tests)",
"cfg(zero_fee_commitment_tests)",
]

[[bench]]
Expand Down
9 changes: 2 additions & 7 deletions benches/payments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,8 @@ fn payment_benchmark(c: &mut Criterion) {
let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
let chain_source = random_chain_source(&bitcoind, &electrsd);

let (node_a, node_b) = setup_two_nodes_with_store(
&chain_source,
false,
true,
false,
common::TestStoreType::Sqlite,
);
let (node_a, node_b) =
setup_two_nodes_with_store(&chain_source, false, false, common::TestStoreType::Sqlite);

let runtime =
tokio::runtime::Builder::new_multi_thread().worker_threads(4).enable_all().build().unwrap();
Expand Down
1 change: 1 addition & 0 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ enum NodeError {
"LnurlAuthFailed",
"LnurlAuthTimeout",
"InvalidLnurl",
"ChainSourceNotSupported",
};

typedef dictionary NodeStatus;
Expand Down
35 changes: 35 additions & 0 deletions scripts/build_electrs.sh

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.

Could you expand on why this needs to be a separate script now? Why not keep using the old script that sets up both bitcoind and electrs?

@tankyleo tankyleo Jul 14, 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.

Codex:

 On your main question: a separate build_electrs.sh file is not strictly necessary, but a separate electrs execution path is. The old combined script plus this condition:

  cache-bitcoind miss || cache-electrs miss

  would always build/download both artifacts when either cache missed. The current split fixes that: download_bitcoind.sh only runs on a bitcoind miss, and build_electrs.sh only runs on an electrs miss.

  So I_d keep the logical split. If you dislike the extra file, the equivalent would be one script with subcommands, e.g. scripts/setup_ci_binary.sh bitcoind and scripts/setup_ci_binary.sh electrs. I would not go back to one
  combined script invoked by the OR condition.

This matches my original intention when I first created this standalone script for the expensive electrs build step.

It looks like we want to avoid building the binary ourselves anyway for easier local development, so I will likely be able to consolidate things again once a binary that supports submitpackage on electrum is served somewhere.

@tnull tnull Jul 14, 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 matches my original intention when I first created this standalone script for the expensive electrs build step.

Okay, yeah, not too strong of an opinion.

It looks like we want to avoid building the binary ourselves anyway for easier local development, so I will likely be able to consolidate things again once a binary that supports submitpackage on electrum is served somewhere.

Well, given how close this PR is it would be nice to land it for 0.8 still, and not wait indefinitely. Hmm, given that the Forgejo migration is already around the corner, further CI changes will be necessary anyways soon. Maybe we could at least provide a convenience wrapper script here, that sets up bitcoind/electrsd and runs the tests? Or even better just do #660 (comment), i.e., leave the current test setup as is and only test 0fc in a separate new file and workflow, so we don't require the custom electrs for all local tests?

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.

See the commit below titled "Isolate testing of 0FC channels to standalone CI"

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#!/bin/bash
Comment thread
benthecarman marked this conversation as resolved.
set -eox pipefail

# Our Esplora-based tests require `electrs` binaries. Here, we
# download the code, build the binaries, and export their location
# via `ELECTRS_EXE` which will be used by the `electrsd` crates in
# our tests.

HOST_PLATFORM="$(rustc --version --verbose | grep "host:" | awk '{ print $2 }')"
ELECTRS_GIT_REPO="https://github.com/tankyleo/blockstream-electrs.git"
ELECTRS_TAG="2026-05-26-electrum-submit-package"

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.

If we do this, we should use a specific commit revision, not point to a general branch.

@tankyleo tankyleo Jun 23, 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.

This is indeed my intention, and I believe the script currently does this.

See the tag here: https://github.com/tankyleo/blockstream-electrs/releases/tag/2026-05-26-electrum-submit-package

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.

Ah, sorry, I took 2026-05-26-electrum-submit-package to be a branch name, not a tag. Would still be good to pin the specific revision hash here rather than a tag.

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 below, the tag remains useful to do a git checkout --depth 1 --branch <tag_name>

ELECTRS_REV="8c06d8010e43f793b1a65f83695ea846e5cd83ed"
if [[ "$HOST_PLATFORM" != *linux* && "$HOST_PLATFORM" != *darwin* ]]; then
printf "\n\n"
echo "Unsupported platform: $HOST_PLATFORM Exiting.."
exit 1
fi

DL_TMP_DIR=$(mktemp -d)
trap 'rm -rf -- "$DL_TMP_DIR"' EXIT

pushd "$DL_TMP_DIR"
git clone --branch "$ELECTRS_TAG" --depth 1 "$ELECTRS_GIT_REPO" blockstream-electrs
cd blockstream-electrs
CURRENT_HEAD=$(git rev-parse HEAD)
if [ "$CURRENT_HEAD" != "$ELECTRS_REV" ]; then
echo "ERROR: HEAD does not match expected commit"
echo "expected: $ELECTRS_REV"
echo "actual: $CURRENT_HEAD"
exit 1
fi
RUSTFLAGS="" cargo build
export ELECTRS_EXE="$DL_TMP_DIR"/blockstream-electrs/target/debug/electrs
chmod +x "$ELECTRS_EXE"
popd
189 changes: 151 additions & 38 deletions src/chain/bitcoind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};

use base64::prelude::BASE64_STANDARD;
use base64::Engine;
use bitcoin::transaction::Version;
use bitcoin::{BlockHash, FeeRate, Network, OutPoint, Transaction, Txid};
use lightning::chain::chaininterface::ConfirmationTarget as LdkConfirmationTarget;
use lightning::chain::{BlockLocator, Listen};
Expand Down Expand Up @@ -41,6 +42,7 @@ use crate::fee_estimator::{
};
use crate::io::utils::update_and_persist_node_metrics;
use crate::logger::{log_bytes, log_debug, log_error, log_info, log_trace, LdkLogger, Logger};
use crate::tx_broadcaster::SortedTransactions;
use crate::types::{ChainMonitor, ChannelManager, DynStore, Sweeper, Wallet};
use crate::{Error, PersistedNodeMetrics};

Expand Down Expand Up @@ -119,6 +121,30 @@ impl BitcoindChainSource {
self.api_client.utxo_source()
}

pub(super) async fn validate_zero_fee_commitments_support(&self) -> Result<(), Error> {
let node_version_result = tokio::time::timeout(
Duration::from_secs(CHAIN_POLLING_TIMEOUT_SECS),
self.api_client.get_node_version(),
)
.await
.map_err(|e| {
log_error!(self.logger, "Failed to get node version: {:?}", e);
Error::ConnectionFailed
})?;

let node_version = node_version_result.map_err(|e| {
log_error!(self.logger, "Failed to get node version: {:?}", e);
Error::ConnectionFailed
})?;

// v26 first shipped the `submitpackage` RPC, but we need v29 to relay ephemeral dust
if node_version < 290000 {
log_error!(self.logger, "Bitcoin backend MUST be greater than or equal to v29");
return Err(Error::ChainSourceNotSupported);
}
Ok(())
}

pub(super) async fn continuously_sync_wallets(
&self, mut stop_sync_receiver: tokio::sync::watch::Receiver<()>,
onchain_wallet: Arc<Wallet>, channel_manager: Arc<ChannelManager>,
Expand Down Expand Up @@ -571,46 +597,59 @@ impl BitcoindChainSource {
Ok(())
}

pub(crate) async fn process_broadcast_package(&self, package: Vec<Transaction>) {
// While it's a bit unclear when we'd be able to lean on Bitcoin Core >v28
// features, we should eventually switch to use `submitpackage` via the
// `rust-bitcoind-json-rpc` crate rather than just broadcasting individual
// transactions.
for tx in &package {
let txid = tx.compute_txid();
let timeout_fut = tokio::time::timeout(
Duration::from_secs(DEFAULT_TX_BROADCAST_TIMEOUT_SECS),
self.api_client.broadcast_transaction(tx),
);
match timeout_fut.await {
Ok(res) => match res {
Ok(id) => {
debug_assert_eq!(id, txid);
log_trace!(self.logger, "Successfully broadcast transaction {}", txid);
},
Err(e) => {
log_error!(self.logger, "Failed to broadcast transaction {}: {}", txid, e);
log_trace!(
self.logger,
"Failed broadcast transaction bytes: {}",
log_bytes!(tx.encode())
);
fn log_broadcast_error(
&self, e: impl core::fmt::Display, txids: &[Txid], txs: &SortedTransactions,
) {
log_error!(self.logger, "Failed to broadcast transaction(s) {:?}: {}", txids, e);
log_trace!(self.logger, "Failed broadcast transaction bytes:");
for tx in txs.iter() {
log_trace!(self.logger, "{}", log_bytes!(tx.encode()));
}
}

pub(crate) async fn process_transaction_broadcast(&self, txs: SortedTransactions) {
let all_txs_are_v3 = txs.iter().all(|tx| tx.version == Version::non_standard(3));
match txs.len() {
2.. if all_txs_are_v3 => {
let txids: Vec<_> = txs.iter().map(|tx| tx.compute_txid()).collect();
let timeout_fut = tokio::time::timeout(
Duration::from_secs(DEFAULT_TX_BROADCAST_TIMEOUT_SECS),
self.api_client.submit_package(&txs),
);
match timeout_fut.await {
Ok(res) => match res {
Ok(result) => {
log_trace!(self.logger, "Successfully broadcast package {:?}", txids);
log_trace!(self.logger, "Successfully broadcast package {}", result);
},
Err(e) => self.log_broadcast_error(e, &txids, &txs),
},
},
Err(e) => {
log_error!(
self.logger,
"Failed to broadcast transaction due to timeout {}: {}",
txid,
e
);
log_trace!(
self.logger,
"Failed broadcast transaction bytes: {}",
log_bytes!(tx.encode())
Err(e) => self.log_broadcast_error(e, &txids, &txs),
}
},
_ => {
for tx in txs.iter() {
let txid = tx.compute_txid();
let timeout_fut = tokio::time::timeout(
Duration::from_secs(DEFAULT_TX_BROADCAST_TIMEOUT_SECS),
self.api_client.broadcast_transaction(tx),
);
},
}
match timeout_fut.await {
Ok(res) => match res {
Ok(id) => {
debug_assert_eq!(id, txid);
log_trace!(
self.logger,
"Successfully broadcast transaction {}",
txid
);
},
Err(e) => self.log_broadcast_error(e, &[txid], &txs),
},
Err(e) => self.log_broadcast_error(e, &[txid], &txs),
}
}
},
}
}
}
Expand Down Expand Up @@ -748,6 +787,31 @@ impl BitcoindClient {
}
}

pub(crate) async fn get_node_version(&self) -> Result<u64, BitcoindClientError> {
match self {
BitcoindClient::Rpc { rpc_client, .. } => {
Self::get_node_version_inner(Arc::clone(rpc_client))
.await
.map_err(BitcoindClientError::Rpc)
},
BitcoindClient::Rest { rpc_client, .. } => {
// Bitcoin Core's REST interface does not support `getnetworkinfo`
// so we use the RPC client.
Self::get_node_version_inner(Arc::clone(rpc_client))
.await
.map_err(BitcoindClientError::Rpc)
},
}
}

async fn get_node_version_inner(rpc_client: Arc<RpcClient>) -> Result<u64, RpcClientError> {
rpc_client.call_method::<serde_json::Value>("getnetworkinfo", &[]).await.and_then(|value| {

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.

Can also look into properly parsing the return value here like how we did for submitpackage let me know

value["version"].as_u64().ok_or(RpcClientError::InvalidData(String::from(
"The version field in the `getnetworkinfo` response should be a u64",
)))
})
}

/// Broadcasts the provided transaction.
pub(crate) async fn broadcast_transaction(
&self, tx: &Transaction,
Expand Down Expand Up @@ -776,6 +840,38 @@ impl BitcoindClient {
rpc_client.call_method::<Txid>("sendrawtransaction", &[tx_json]).await
}

/// Submits the provided package
pub(crate) async fn submit_package(
&self, package: &SortedTransactions,
) -> Result<String, BitcoindClientError> {
match self {
BitcoindClient::Rpc { rpc_client, .. } => {
Self::submit_package_inner(Arc::clone(rpc_client), package)
.await
.map_err(BitcoindClientError::Rpc)
},
BitcoindClient::Rest { rpc_client, .. } => {
// Bitcoin Core's REST interface does not support submitting packages
// so we use the RPC client.
Self::submit_package_inner(Arc::clone(rpc_client), package)
.await
.map_err(BitcoindClientError::Rpc)
},
}
}

async fn submit_package_inner(
rpc_client: Arc<RpcClient>, package: &SortedTransactions,
) -> Result<String, RpcClientError> {
let package_serialized: Vec<_> =
package.iter().map(|tx| bitcoin::consensus::encode::serialize_hex(tx)).collect();
let package_json = serde_json::json!(package_serialized);
rpc_client
.call_method::<SubmitPackageResponse>("submitpackage", &[package_json])
.await
.map(|response| response.0)
}

/// Retrieve the fee estimate needed for a transaction to begin
/// confirmation within the provided `num_blocks`.
pub(crate) async fn get_fee_estimate_for_target(
Expand Down Expand Up @@ -1327,6 +1423,23 @@ impl TryInto<GetMempoolEntryResponse> for JsonResponse {
}
}

pub struct SubmitPackageResponse(String);

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.

Should we even return the full response string here rather than ()?

@tankyleo tankyleo Jun 25, 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.

See the esplora and electrum chain sources, I prefer to log the full response at trace even in the success case for debugging purposes. We get information on replaced transactions, transactions already in the mempool, and transactions freshly accepted into the mempool.


impl TryInto<SubmitPackageResponse> for JsonResponse {
type Error = String;
fn try_into(self) -> Result<SubmitPackageResponse, String> {
let response = self.0.to_string();
let res = self.0.as_object().ok_or("Failed to parse submitpackage response".to_string())?;

match res["package_msg"].as_str() {
Some("success") => Ok(SubmitPackageResponse(response)),
Some(_) | None => {
return Err(response);
},
}
}
}

#[derive(Debug, Clone)]
pub(crate) struct MempoolEntry {
/// The transaction id
Expand Down
Loading
Loading