Check spend status in bitcoin poller

[edouardparis]

Add two new columns:

• blocktime: timestamp of the block containing the transaction funding the coin.
• spent_at: timestamp of the block containing the transaction spending the coin.

Update the coin spent_at when the spend transaction is confirmed

[darosior]

Cool. I've found one "bug" (an edge case that isn't covered) in the poller, and the rest is mostly nits. Let me know if you prefer to ignore them. Though i do feel somewhat strongly about (in order of preference):

1. Removing the arbitrary 6-confs rule. I added it in Revault since we had presigned transactions but here i think assuming it's confirmed with a single conf is fine for now.
2. The interface for marking a spend as confirmed / the name of some of the trait methods introduced

[darosior]

Do you need to clone()? Can't you just as_ref()?

[edouardparis]

There is an immutable borrow on the cache.get, but you are right that it is a certainly better way to do it

[darosior]

I'm responding just to clarify, but no need to address this

I don't understand. Why not just iterate over the vector and copy the txid only if it happens to be necessary, instead of always reallocate the entire vector?
Like:

diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs
index 0c0f9c1..b171054 100644
--- a/src/bitcoin/mod.rs
+++ b/src/bitcoin/mod.rs
@@ -176,12 +176,12 @@ impl BitcoinInterface for d::BitcoinD {
                         spent.push((*op, *txid, tx.block_time.expect("Spend is confirmed")))
                     }
                 } else if !tx.conflicting_txs.is_empty() {
-                    for txid in tx.conflicting_txs.clone() {
-                        let tx: Option<&d::GetTxRes> = match cache.get(&txid) {
+                    for txid in &tx.conflicting_txs {
+                        let tx: Option<&d::GetTxRes> = match cache.get(txid) {
                             Some(tx) => tx.as_ref(),
                             None => {
                                 let tx = self.get_transaction(&txid);
-                                txs_to_cache.push((txid, tx));
+                                txs_to_cache.push((*txid, tx));
                                 txs_to_cache.last().unwrap().1.as_ref()
                             }
                         };
@@ -190,7 +190,7 @@ impl BitcoinInterface for d::BitcoinD {
                                 if block_height > 1 {
                                     spent.push((
                                         *op,
-                                        txid,
+                                        *txid,
                                         tx.block_time.expect("Spend is confirmed"),
                                     ))
                                 }

[darosior]

Almost ACK. I'm just confused by the check that the block_height of the spending transaction be higher than 1. Feel free to ignore the rest.

[darosior]

I'm responding just to clarify, but no need to address this

I don't understand. Why not just iterate over the vector and copy the txid only if it happens to be necessary, instead of always reallocate the entire vector?
Like:

diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs
index 0c0f9c1..b171054 100644
--- a/src/bitcoin/mod.rs
+++ b/src/bitcoin/mod.rs
@@ -176,12 +176,12 @@ impl BitcoinInterface for d::BitcoinD {
                         spent.push((*op, *txid, tx.block_time.expect("Spend is confirmed")))
                     }
                 } else if !tx.conflicting_txs.is_empty() {
-                    for txid in tx.conflicting_txs.clone() {
-                        let tx: Option<&d::GetTxRes> = match cache.get(&txid) {
+                    for txid in &tx.conflicting_txs {
+                        let tx: Option<&d::GetTxRes> = match cache.get(txid) {
                             Some(tx) => tx.as_ref(),
                             None => {
                                 let tx = self.get_transaction(&txid);
-                                txs_to_cache.push((txid, tx));
+                                txs_to_cache.push((*txid, tx));
                                 txs_to_cache.last().unwrap().1.as_ref()
                             }
                         };
@@ -190,7 +190,7 @@ impl BitcoinInterface for d::BitcoinD {
                                 if block_height > 1 {
                                     spent.push((
                                         *op,
-                                        txid,
+                                        *txid,
                                         tx.block_time.expect("Spend is confirmed"),
                                     ))
                                 }

[darosior]

ACK 172cda1