Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the tracking of immature coinbase deposits #578

Merged
merged 7 commits into from
Aug 1, 2023
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
1 change: 1 addition & 0 deletions doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ This command does not take any parameter for now.
| `outpoint` | string | Transaction id and output index of this coin. |
| `block_height` | int or null | Block height the transaction was confirmed at, or `null`. |
| `spend_info` | object | Information about the transaction spending this coin. See [Spending transaction info](#spending_transaction_info). |
| `is_immature` | bool | Whether this coin was created by a coinbase transaction that is still immature. |


##### Spending transaction info
Expand Down
23 changes: 22 additions & 1 deletion src/bitcoin/d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,7 @@ pub struct LSBlockEntry {
pub block_height: Option<i32>,
pub address: bitcoin::Address<address::NetworkUnchecked>,
pub parent_descs: Vec<descriptor::Descriptor<descriptor::DescriptorPublicKey>>,
pub is_immature: bool,
}

impl From<&Json> for LSBlockEntry {
Expand Down Expand Up @@ -1150,12 +1151,19 @@ impl From<&Json> for LSBlockEntry {
})
.expect("bitcoind can't give invalid descriptors");

let is_immature = json
.get("category")
.and_then(Json::as_str)
.expect("must be present")
== "immature";

LSBlockEntry {
outpoint,
amount,
block_height,
address,
parent_descs,
is_immature,
}
}
}
Expand Down Expand Up @@ -1183,7 +1191,7 @@ impl From<Json> for LSBlockRes {
.get("category")
.and_then(Json::as_str)
.expect("must be present");
if category == "receive" || category == "generate" {
if ["receive", "generate", "immature"].contains(&category) {
let lsb_entry: LSBlockEntry = j.into();
Some(lsb_entry)
} else {
Expand All @@ -1201,6 +1209,8 @@ pub struct GetTxRes {
pub conflicting_txs: Vec<bitcoin::Txid>,
pub block: Option<Block>,
pub tx: bitcoin::Transaction,
pub is_coinbase: bool,
pub confirmations: i32,
}

impl From<Json> for GetTxRes {
Expand Down Expand Up @@ -1238,10 +1248,21 @@ impl From<Json> for GetTxRes {
let bytes = Vec::from_hex(hex).expect("bitcoind returned a wrong transaction format");
let tx: bitcoin::Transaction = bitcoin::consensus::encode::deserialize(&bytes)
.expect("bitcoind returned a wrong transaction format");
let is_coinbase = json
.get("generated")
.and_then(Json::as_bool)
.unwrap_or(false);
let confirmations = json
.get("confirmations")
.and_then(Json::as_i64)
.expect("Must be present in the response") as i32;

GetTxRes {
conflicting_txs: conflicting_txs.unwrap_or_default(),
block,
tx,
is_coinbase,
confirmations,
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/bitcoin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use std::{fmt, sync};

use miniscript::bitcoin::{self, address};

const COINBASE_MATURITY: i32 = 100;

/// Information about a block
#[derive(Debug, Clone, Eq, PartialEq, Copy)]
pub struct Block {
Expand Down Expand Up @@ -146,6 +148,7 @@ impl BitcoinInterface for d::BitcoinD {
block_height,
address,
parent_descs,
is_immature,
} = entry;
if parent_descs
.iter()
Expand All @@ -156,6 +159,7 @@ impl BitcoinInterface for d::BitcoinD {
amount,
block_height,
address,
is_immature,
})
} else {
None
Expand Down Expand Up @@ -184,6 +188,11 @@ impl BitcoinInterface for d::BitcoinD {

// If the transaction was confirmed, mark the coin as such.
if let Some(block) = res.block {
// Do not mark immature coinbase deposits as confirmed until they become mature.
if res.is_coinbase && res.confirmations < COINBASE_MATURITY {
log::debug!("Coin at '{}' comes from an immature coinbase transaction with {} confirmations. Not marking it as confirmed for now.", op, res.confirmations);
continue;
}
confirmed.push((*op, block.height, block.time));
continue;
}
Expand Down Expand Up @@ -409,4 +418,5 @@ pub struct UTxO {
pub amount: bitcoin::Amount,
pub block_height: Option<i32>,
pub address: bitcoin::Address<address::NetworkUnchecked>,
pub is_immature: bool,
}
4 changes: 4 additions & 0 deletions src/bitcoin/poller/looper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ struct UpdatedCoins {
// or spent.
// NOTE: A coin may be updated multiple times at once. That is, a coin may be received, confirmed,
// and spent in a single poll.
// NOTE: Coinbase transaction deposits are very much an afterthought here. We treat them as
// unconfirmed until the CB tx matures.
fn update_coins(
bit: &impl BitcoinInterface,
db_conn: &mut Box<dyn DatabaseConnection>,
Expand All @@ -42,6 +44,7 @@ fn update_coins(
outpoint,
amount,
address,
is_immature,
Copy link
Member Author

Choose a reason for hiding this comment

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

This may trigger the assertion and the SQL constraint. My bad. See #1001.

..
} = utxo;
// We can only really treat them if we know the derivation index that was used.
Expand All @@ -66,6 +69,7 @@ fn update_coins(
if !curr_coins.contains_key(&utxo.outpoint) {
let coin = Coin {
outpoint,
is_immature,
amount,
derivation_index,
is_change,
Expand Down
38 changes: 38 additions & 0 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub enum CommandError {
InvalidFeerate(/* sats/vb */ u64),
UnknownOutpoint(bitcoin::OutPoint),
AlreadySpent(bitcoin::OutPoint),
ImmatureCoinbase(bitcoin::OutPoint),
Address(bitcoin::address::Error),
InvalidOutputValue(bitcoin::Amount),
InsufficientFunds(
Expand Down Expand Up @@ -77,6 +78,7 @@ impl fmt::Display for CommandError {
Self::NoOutpoint => write!(f, "No provided outpoint. Need at least one."),
Self::InvalidFeerate(sats_vb) => write!(f, "Invalid feerate: {} sats/vb.", sats_vb),
Self::AlreadySpent(op) => write!(f, "Coin at '{}' is already spent.", op),
Self::ImmatureCoinbase(op) => write!(f, "Coin at '{}' is from an immature coinbase transaction.", op),
Self::UnknownOutpoint(op) => write!(f, "Unknown outpoint '{}'.", op),
Self::Address(e) => write!(
f,
Expand Down Expand Up @@ -298,6 +300,7 @@ impl DaemonControl {
block_info,
spend_txid,
spend_block,
is_immature,
..
} = coin;
let spend_info = spend_txid.map(|txid| LCSpendInfo {
Expand All @@ -310,6 +313,7 @@ impl DaemonControl {
outpoint,
block_height,
spend_info,
is_immature,
}
})
.collect();
Expand Down Expand Up @@ -349,6 +353,10 @@ impl DaemonControl {
if coin.is_spent() {
return Err(CommandError::AlreadySpent(*op));
}
if coin.is_immature {
return Err(CommandError::ImmatureCoinbase(*op));
}

// Fetch the transaction that created it if necessary
if !spent_txs.contains_key(op) {
let tx = self
Expand Down Expand Up @@ -840,6 +848,8 @@ pub struct ListCoinsEntry {
pub block_height: Option<i32>,
/// Information about the transaction spending this coin.
pub spend_info: Option<LCSpendInfo>,
/// Whether this coin was created by a coinbase transaction that is still immature.
pub is_immature: bool,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down Expand Up @@ -976,6 +986,7 @@ mod tests {
let mut db_conn = control.db().lock().unwrap().connection();
db_conn.new_unspent_coins(&[Coin {
outpoint: dummy_op,
is_immature: false,
block_info: None,
amount: bitcoin::Amount::from_sat(100_000),
derivation_index: bip32::ChildNumber::from(13),
Expand Down Expand Up @@ -1081,6 +1092,7 @@ mod tests {
};
db_conn.new_unspent_coins(&[Coin {
outpoint: dummy_op_dup,
is_immature: false,
block_info: None,
amount: bitcoin::Amount::from_sat(400_000),
derivation_index: bip32::ChildNumber::from(42),
Expand All @@ -1095,6 +1107,26 @@ mod tests {
)))
);

// Can't create a transaction that spends an immature coinbase deposit.
let imma_op = bitcoin::OutPoint::from_str(
"4753a1d74c0af8dd0a0f3b763c14faf3bd9ed03cbdf33337a074fb0e9f6c7810:0",
)
.unwrap();
db_conn.new_unspent_coins(&[Coin {
outpoint: imma_op,
is_immature: true,
block_info: None,
amount: bitcoin::Amount::from_sat(100_000),
derivation_index: bip32::ChildNumber::from(13),
is_change: false,
spend_txid: None,
spend_block: None,
}]);
assert_eq!(
control.create_spend(&destinations, &[imma_op], 1_001),
Err(CommandError::ImmatureCoinbase(imma_op))
);

ms.shutdown();
}

Expand Down Expand Up @@ -1127,6 +1159,7 @@ mod tests {
db_conn.new_unspent_coins(&[
Coin {
outpoint: dummy_op_a,
is_immature: false,
block_info: None,
amount: bitcoin::Amount::from_sat(100_000),
derivation_index: bip32::ChildNumber::from(13),
Expand All @@ -1136,6 +1169,7 @@ mod tests {
},
Coin {
outpoint: dummy_op_b,
is_immature: false,
block_info: None,
amount: bitcoin::Amount::from_sat(115_680),
derivation_index: bip32::ChildNumber::from(34),
Expand Down Expand Up @@ -1303,6 +1337,7 @@ mod tests {
// Deposit 1
Coin {
is_change: false,
is_immature: false,
outpoint: OutPoint {
txid: deposit1.txid(),
vout: 0,
Expand All @@ -1316,6 +1351,7 @@ mod tests {
// Deposit 2
Coin {
is_change: false,
is_immature: false,
outpoint: OutPoint {
txid: deposit2.txid(),
vout: 0,
Expand All @@ -1329,6 +1365,7 @@ mod tests {
// This coin is a change output.
Coin {
is_change: true,
is_immature: false,
outpoint: OutPoint::new(spend_tx.txid(), 1),
block_info: Some(BlockInfo { height: 3, time: 3 }),
spend_block: None,
Expand All @@ -1339,6 +1376,7 @@ mod tests {
// Deposit 3
Coin {
is_change: false,
is_immature: false,
outpoint: OutPoint {
txid: deposit3.txid(),
vout: 0,
Expand Down
6 changes: 6 additions & 0 deletions src/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ pub trait DatabaseConnection {
fn remove_coins(&mut self, coins: &[bitcoin::OutPoint]);

/// Mark a set of coins as being confirmed at a specified height and block time.
/// NOTE: if the coin comes from an immature coinbase transaction, this will mark it as mature.
/// Immature coinbase deposits must not be confirmed before they are 100 blocks deep in the
/// chain.
fn confirm_coins(&mut self, outpoints: &[(bitcoin::OutPoint, i32, u32)]);

/// Mark a set of coins as being spent by a specified txid of a pending transaction.
Expand Down Expand Up @@ -281,6 +284,7 @@ impl From<DbBlockInfo> for BlockInfo {
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct Coin {
pub outpoint: bitcoin::OutPoint,
pub is_immature: bool,
pub block_info: Option<BlockInfo>,
pub amount: bitcoin::Amount,
pub derivation_index: bip32::ChildNumber,
Expand All @@ -293,6 +297,7 @@ impl std::convert::From<DbCoin> for Coin {
fn from(db_coin: DbCoin) -> Coin {
let DbCoin {
outpoint,
is_immature,
block_info,
amount,
derivation_index,
Expand All @@ -303,6 +308,7 @@ impl std::convert::From<DbCoin> for Coin {
} = db_coin;
Coin {
outpoint,
is_immature,
block_info: block_info.map(BlockInfo::from),
amount,
derivation_index,
Expand Down
Loading
Loading