OpenBazaar / multiwallet

API based multi-cryptocurrency wallet
MIT License
72 stars 41 forks source link

AddressBalance command fetches balance for the given coin-type/address pair #40

Closed andreygrehov closed 4 years ago

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 240


Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/wallet.go 0 10 0.0%
litecoin/wallet.go 0 10 0.0%
zcash/wallet.go 0 10 0.0%
<!-- Total: 0 30 0.0% -->
Totals Coverage Status
Change from base Build 238: -0.4%
Covered Lines: 2009
Relevant Lines: 3776

💛 - Coveralls
andreygrehov commented 5 years ago

@placer14 good call. I'm not sure if extraction will work, wallet.Utxo doesn't know how to convert its ScriptPubkey to btcutil.Address.

https://github.com/OpenBazaar/multiwallet/pull/40/files#diff-d06015dd60330e13eb491a605f0bed80R184

Any ideas?

A callback approach could do the job I believe. Something along these lines:

func (w *BitcoinWallet) AddressBalance(addr btc.Address) (confirmed, unconfirmed int64) {
    utxos, _ := w.db.Utxos().Filter(func(utxo wi.Utxo) bool {
        utxoAddr, _ := w.ScriptToAddress(utxo.ScriptPubkey)
        return addr.String() == utxoAddr.String()
    })
    txns, _ := w.db.Txns().GetAll(false)
    return util.CalcBalance(filteredUtxos, txns)
}

It's not that shorter though.

placer14 commented 5 years ago

I like the callback approach. Unfortunately that shifts the Utxo interface in the wallet-interface. You could make that interface local without impacting external dependancies, probably, but I'm not sure it's worth all of that infrastructure just to collapse this behavior. Let's call it a wash and leave it as is. If we can get a blessing from @cpacia then I'd be fine with merging this.

andreygrehov commented 5 years ago

@placer14 cool, thanks. We'll need to get another PR for go-ethwallet before merging this one in.

andreygrehov commented 5 years ago

@cpacia does this PR make any sense? If not, we should close it out. Otherwise, let me know.