lbryio / lbcwallet

A secure lbry wallet daemon written in Go
ISC License
6 stars 8 forks source link

`sendfrom` "imported" account rejected #18

Closed roylee17 closed 2 years ago

roylee17 commented 2 years ago
lbcctl --wallet --skipverify sendfrom imported bGwBGtRMwxxE3AZWmr35KvujyZLKtzBBDA 0.0001

-32603: unmatched backend error: -25: TX rejected: failed to validate input 74630c94b7e68af237a52b945c937ebd9b730193c36fb89b0caedfe7a9a14072:0 which references output 330e730750641f4fc988bc0246bc9c53118abf396737c0be584cfb6faace560f:0 - index 0 is invalid for stack size 0 (input witness [], input script bytes , prev output script bytes 76a9141d889780c368db5d2e8c1a20d7c4d29f497213d088ac)
roylee17 commented 2 years ago

Root cause: Currently, all imported keys/addresses are assumed watch-only, and were skipped from adding into transaction inputs.

https://github.com/lbryio/lbcwallet/blob/d1700f2658b653f26749fe7f86a5afb7ab3a491f/waddrmgr/manager.go#L418-L424

    // Assume the default imported account has no private keys.
    //
    // TODO: Actually check whether it does.
    if account == ImportedAddrAccount {
        return true, nil
    }

https://github.com/lbryio/lbcwallet/blob/d1700f2658b653f26749fe7f86a5afb7ab3a491f/wallet/createtx.go#L220-L235

if !watchOnly {
    err = tx.AddAllInputScripts(
        secretSource{w.Manager, addrmgrNs},
    )
    if err != nil {
        return err
    }

    err = validateMsgTx(
        tx.Tx, tx.PrevScripts, tx.PrevInputValues,
    )
    if err != nil {
        return err
    }
}

An easy solution is to check the existence of the private key for each address. But it makes the code flow unnecessarily convoluted.

I'm leaning toward reserving a"imported-watchonly" account (2^32-2) dedicated for imported public keys and addresses. Leave the "imported" account for those imported using importprivkey

roylee17 commented 2 years ago

The current master includes a hotfix, which sacrifices watch-only address support in favor of imported addresses.

Earlier we have PR #23 to address the issue by implementing a watch-only account to house all watch-only addresses. However, using fixed dedicated accounts to house import keys can be awkward, and have many places to handle those accounts as corner cases.

We decided to implement proper multi-account support first. Later, we can allow users to decide which account(s) will be used for imported keys.