fedimint / fedimint

Federated E-Cash Mint
https://fedimint.org/
MIT License
554 stars 211 forks source link

fix: recognize change outputs for bitcoind backends without txindex #5307

Closed bradleystachurski closed 3 weeks ago

bradleystachurski commented 1 month ago

Fixes https://github.com/fedimint/fedimint/issues/5298

Peers using bitcoind without txindex will not recognize change outputs. This PR introduces is_tx_in_block to IBitcoindRpc, which uses getblock for bitcoind that doesn't require txindex.

For pruned nodes, calls to getblock will return an error if the block data has been pruned (see: https://github.com/bitcoin/bitcoin/blob/v0.11.0/doc/release-notes.md#block-file-pruning). The minimum allowed for pruned nodes is 550MB, which is ~2 days/288 blocks assuming average block size ~1.9MB. If a peer attempts to sync blocks that have been pruned while there are outstanding pending transactions, the underlying call to getblock will fail.


This change will prevent further issues but does not resolve bad state for federations that have not recognized pending change transactions. I'm still investigating a simple fix along with ensuring we can use the recoverytool for impacted federations.

bradleystachurski commented 3 weeks ago

~@elsirion I'd like to do more investigation for how pruned nodes will behave before moving out of draft, but this is likely close if you want to take a quick look at the approach.~

~According to the docs, the min pruned value is 550MB, which is 2 days/288 blocks assuming average block size 1.9MB. If a peer attempts to sync blocks that have been pruned while there are outstanding pending transactions, the underlying call to getblock will fail.~

Edit: updated PR description for clarity

bradleystachurski commented 3 weeks ago

pr review club: out-of-scope for this pr, but it would be useful to not rely on network calls inside of process consensus item. For syncing blocks, this would mean creating an in-memory queue of blocks with txids as a separate background task, then when syncing a block after the finality delay in process consensus item we would read from the in-memory queue instead of relying on a network call. The in-memory queue would have a depth of tip - finality delay.

A consideration for this approach is handling reorgs of the blocks in the in-memory queue. A naive implementation is to verify the most recent hash in the in-memory queue is still in the chain whenever we observer a new tip. If there's a divergence, simply refetch the entire queue instead of trying to find the most recent ancestor.

fedimint-backports commented 3 weeks ago

Backport failed for releases/v0.3, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin releases/v0.3
git worktree add -d .worktree/backport-5307-to-releases/v0.3 origin/releases/v0.3
cd .worktree/backport-5307-to-releases/v0.3
git switch --create backport-5307-to-releases/v0.3
git cherry-pick -x 6e8290f6cd6c035ef2c2b281086d9be2bdf99e27 b4090b789b09a2f9f7c581283d84f432aa2958ce d9c7e5a204c5f6d43764e895393e15ad733d3e4d