Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wallet: add "accountPath" attribute to handle edges cases with watch-only accountKeys #689

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

wallet: add "accountPath" attribute to handle edges cases with watch-only accountKeys #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/wallet/account.js
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ const Path = require('./path');
const common = require('./common');
const Script = require('../script/script');
const WalletKey = require('./walletkey');
const HDCommon = require('../hd/common');
const {HDPublicKey} = require('../hd/hd');

/**
Expand Down Expand Up @@ -40,6 +41,7 @@ class Account {
this.wid = 0;
this.id = null;
this.accountIndex = 0;
this.accountPath = 0;
this.name = null;
this.initialized = false;
this.witness = wdb.options.witness === true;
Expand Down Expand Up @@ -146,6 +148,11 @@ class Account {

this.accountKey = options.accountKey;

// for watch-only wallets, accountPath is derived from accountKey
this.accountPath = this.watchOnly ?
this.accountKey.childIndex ^ HDCommon.HARDENED :
this.accountIndex;

if (this.n > 1)
this.type = Account.types.MULTISIG;

Expand Down Expand Up @@ -789,6 +796,7 @@ class Account {
m: this.m,
n: this.n,
accountIndex: this.accountIndex,
accountPath: this.accountPath,
receiveDepth: this.receiveDepth,
changeDepth: this.changeDepth,
nestedDepth: this.nestedDepth,
Expand Down Expand Up @@ -821,6 +829,7 @@ class Account {
m: this.m,
n: this.n,
accountIndex: this.accountIndex,
accountPath: this.accountPath,
receiveDepth: this.receiveDepth,
changeDepth: this.changeDepth,
nestedDepth: this.nestedDepth,
Expand Down
6 changes: 5 additions & 1 deletion lib/wallet/path.js
View file
Open in desktop
Expand Up @@ -32,6 +32,7 @@ class Path {

this.name = null; // Passed in by caller.
this.account = 0;
this.accountPath = 0;

this.type = Address.types.PUBKEYHASH;
this.version = -1;
Expand Down Expand Up @@ -60,6 +61,7 @@ class Path {

this.name = options.name;
this.account = options.account;
this.accountPath = options.account;
this.branch = options.branch;
this.index = options.index;

Expand Down Expand Up @@ -238,6 +240,7 @@ class Path {
this.keyType = Path.types.ADDRESS;
this.name = account.name;
this.account = account.accountIndex;
this.accountPath = account.account;
this.version = address.version;
this.type = address.type;
this.hash = address.getHash();
Expand All @@ -264,7 +267,7 @@ class Path {
if (this.keyType !== Path.types.HD)
return null;

return `m/${this.account}'/${this.branch}/${this.index}`;
return `m/${this.accountPath}'/${this.branch}/${this.index}`;
}

/**
Expand All @@ -285,6 +288,7 @@ class Path {
return {
name: this.name,
account: this.account,
accountPath: this.accountPath || this.account,
change: this.branch === 1,
derivation: this.toPath()
};
Expand Down
7 changes: 6 additions & 1 deletion lib/wallet/walletdb.js
View file
Open in desktop
Expand Up @@ -22,6 +22,7 @@ const Path = require('./path');
const common = require('./common');
const Wallet = require('./wallet');
const Account = require('./account');
const HDCommon = require('../hd/common');
const Outpoint = require('../primitives/outpoint');
const layouts = require('./layout');
const records = require('./records');
Expand Down Expand Up @@ -1117,7 +1118,9 @@ class WalletDB extends EventEmitter {
assert(data);

const account = Account.fromRaw(this, data);
const childIndex = account.accountKey.childIndex;

account.accountPath = childIndex ^ HDCommon.HARDENED;
Copy link
Member

@pinheadmz pinheadmz Feb 5, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the account key's child index is already hardened, and therefore already > 0x80000000
so I think this xor is redundant. And actually if the imported xpub for whatever reason is not hardened, we probably need to leave it that way

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns the expected value for hardened keys, which should always be the case with xpubs. (eg, returns accountPath '1' instead of '2147483649`). Perhaps there are edge cases of non-hardened xpubs, but not sure how to handle that one...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see its an xor not or so the constant gets cancelled out 👍

account.accountIndex = index;
account.name = name;

Expand Down Expand Up @@ -1250,7 +1253,9 @@ class WalletDB extends EventEmitter {
if (!path)
return null;

path.name = await this.getAccountName(wid, path.account);
const account = await this.getAccount(wid, path.account);
path.name = account.name;
path.accountPath = account.accountPath;
assert(path.name);

return path;
Expand Down
6 changes: 6 additions & 0 deletions lib/wallet/walletkey.js
View file
Open in desktop
Expand Up @@ -32,6 +32,7 @@ class WalletKey extends KeyRing {

this.name = null;
this.account = -1;
this.accountPath = -1;
this.branch = -1;
this.index = -1;
}
Expand All @@ -45,6 +46,7 @@ class WalletKey extends KeyRing {
return {
name: this.name,
account: this.account,
accountPath: this.accountPath || this.account,
branch: this.branch,
index: this.index,
witness: this.witness,
Expand All @@ -71,6 +73,7 @@ class WalletKey extends KeyRing {
this.keyType = Path.types.HD;
this.name = account.name;
this.account = account.accountIndex;
this.accountPath = account.accountPath;
this.branch = branch;
this.index = index;
this.witness = account.witness;
Expand Down Expand Up @@ -107,6 +110,7 @@ class WalletKey extends KeyRing {
this.keyType = Path.types.KEY;
this.name = account.name;
this.account = account.accountIndex;
this.accountPath = account.accountIndex;
this.witness = account.witness;
return this.fromRaw(data);
}
Expand Down Expand Up @@ -134,6 +138,7 @@ class WalletKey extends KeyRing {
this.keyType = Path.types.KEY;
this.name = account.name;
this.account = account.accountIndex;
this.accountPath = account.accountIndex;
this.witness = account.witness;
return this.fromOptions(ring);
}
Expand All @@ -159,6 +164,7 @@ class WalletKey extends KeyRing {

path.name = this.name;
path.account = this.account;
path.accountPath = this.account;

switch (this.keyType) {
case Path.types.HD:
Expand Down