`sendfrom` "imported" account rejected #18

New issue

Closed

opened 2022-08-03 04:45:44 +02:00 by roylee17 · 2 comments

roylee17 commented

2022-08-03 04:45:44 +02:00

(Migrated from github.com)

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)

``` 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

2022-08-03 08:31:20 +02:00

(Migrated from github.com)

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

d1700f2658/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
	}

d1700f2658/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

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

2022-08-29 09:09:33 +02:00

(Migrated from github.com)

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.

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.