Enhance bdk chain structures

[vladimirfomene]

Description

Fixes #1061

Changelog notice

• Move WalletUpdate to the wallet module
• Remove ForEachTxout trait completely
• Refactor ElectrumExt to not use WalletUpdate.

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

[LLFourn]

What's the motivation for removing ForEachTxOut? I think it serves a purpose in that it allows you to scan several things that are bunches of txouts (including things that are not transactions).

[evanlinjin]

What's the motivation for removing ForEachTxOut? I think it serves a purpose in that it allows you to scan several things that are bunches of txouts (including things that are not transactions).

This was my suggestion and I wrote it on the ticket. I don't think it is needed? The Indexer trait has the method Indexer::index_txout(&mut self, outpoint: OutPoint, txout: &TxOut) -> Self::ChangeSet, and we can have another method:

pub trait Indexer {
    /* ... */

    fn index_txouts<'a>(
        &mut self,
        txouts: impl IntoIterator<Item = (OutPoint, &'a TxOut)>,
    ) -> Self::ChangeSet
    where
        Self::ChangeSet: Append + Default,
    {
        let mut changeset = Self::ChangeSet::default();
        for (outpoint, txout) in txouts {
            Append::append(&mut changeset, self.index_txout(outpoint, txout));
        }
        changeset
    }
}

Now anything that implements IntoIterator<Item = (OutPoint, &TxOut)> will work.

[vladimirfomene]

Will rebase this once MSRV fixes make it to master.

[evanlinjin]

Thanks for the work so far. Sorry about my incremental reviews.

[LLFourn]

What's the motivation for removing ForEachTxOut? I think it serves a purpose in that it allows you to scan several things that are bunches of txouts (including things that are not transactions).

This was my suggestion and I wrote it on the ticket. I don't think it is needed? The Indexer trait has the method Indexer::index_txout(&mut self, outpoint: OutPoint, txout: &TxOut) -> Self::ChangeSet, and we can have another method:

So if I want to index a Block what should I do?. I can iterate manually over every TxOut and call this but that seems much more cumbersome than it needs to be?

[vladimirfomene]

@LLFourn I'm thinking you will use index_tx here instead of index_txout.

[LLFourn]

@LLFourn I'm thinking you will use index_tx here instead of index_txout.

Understood. I think might be as easy as we need to make it. Thanks.

[notmandatory]

ACK c56728f

I confirmed example_electrum and example_esplora scan and sync commands still work. Used my test signet descriptors:

cargo run -- --network signet "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/0/*)" "wpkh(tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L/84'/1'/0'/1/*)" scan

[notmandatory]

One small nit, it looks like most of the commits include breaking changes so commit messages should include a ! to denote commit breaks old APIs. Doesn't need to be fixed for this PR but to keep in mind for future.

[evanlinjin]

ACK c56728f

[LLFourn]

Hey @vladimirfomene thanks for doing this refactoring.

I've pointed out a bunch of ways we can simplify things in both wallet types and electrum_ext. Many of the changes requested are not to do with this PR but I think this is the place to do them.

[edit] I also pushed 4104206

[LLFourn]

This LGTM! ACK ee01b0f

left a couple of small comments.

[evanlinjin]

Mostly nits and documentation suggestions.

[evanlinjin]

ACK 1ff806c