[wallet]: only select BIP-86 script keys for channel funding, add flag to FundVirtualPsbt

[guggero]

Fixes a bug that caused channel funding transactions to be mistakenly used for coin selection, even though they are known to be script path only spend assets.

[coveralls]

Pull Request Test Coverage Report for Build 10697119132

Details

• 5 of 33 (15.15%) changed or added relevant lines in 6 files are covered.
• 16 unchanged lines in 4 files lost coverage.
• Overall coverage decreased (-0.007%) to 40.175%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
tapchannel/aux_funding_controller.go	0	1	0.0%
tapfreighter/chain_porter.go	0	1	0.0%
tapfreighter/wallet.go	0	4	0.0%
rpcserver.go	0	22	0.0%

Files with Coverage Reduction	New Missed Lines	%
tapchannel/aux_funding_controller.go	1	0.0%
tapgarden/caretaker.go	4	68.87%
commitment/tap.go	4	83.91%
tapdb/multiverse.go	7	60.32%

Totals
Change from base Build 10628247677:	-0.007%
Covered Lines:	23981
Relevant Lines:	59692

---

💛 - Coveralls

[Roasbeef]

Nice fix!

[Roasbeef]

I think we can make this diff/change and future call sites more readable if we introduce an enum early here instead of a bool.

It could be something like (random names, can def be improved):

type CoinSelectType uint8

const (
    DefaultCoinSelect CoinSelectType
    Bip86Only
    ScriptTreesAllowed 
)

Then we can make DefaultCoinSelect = Bip86Only when parsing, which is typically the behavior you want unless doing something fancy.

[guggero]

Makes sense! Changed the type. But had to keep the default value at ScriptTreesAllowed to not break existing users of the FundVirtualPsbts RPC call (which is primarily used for doing script sends, at least in our itests).

[jharveyb]

LGTM, AFAICT the default for FundVirtualPsbt will be the same but FundAddressSend will be BIP86 only.

Would be nice to have less negations in the path from the enum to the actual DB filter, but minor nit / non-blocking.

[guggero]

have less negations in the path from the enum to the actual DB filter

I'm not sure what you mean. Are you referencing the naming/wording from "only" and "allowed" that is later mapped to a single boolean?

[Roasbeef]

LGTM 📈