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.

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

Fix: Get account by index instead of name #1135

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions bin/bwallet-cli
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,12 @@ class CLI {
async getAccount() {
const acct = this.config.str([0, 'account']);
const account = await this.wallet.getAccount(acct);
this.log(account);
}

async getAccountByIndex() {
const index = this.config.uint([0, 'accountIndex']);
const account = await this.wallet.getAccountIndex(index);
this.log(account);
}

Expand Down Expand Up @@ -542,6 +547,11 @@ class CLI {
this.argv.shift();
await this.getAccount();
break;
case 'accountIndex':
if (this.argv[0] === 'get')
this.argv.shift();
await this.getAccountByIndex();
break;
case 'address':
await this.createAddress();
break;
Expand Down Expand Up @@ -608,6 +618,7 @@ class CLI {
this.log(' $ abandon [hash]: Abandon a transaction.');
this.log(' $ account create [account-name]: Create account.');
this.log(' $ account get [account-name]: Get account details.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please specify that this method gets account by name.

this.log(' $ accountIndex get [account-index]: Get account details by index.');
this.log(' $ account list: List account names.');
this.log(' $ address: Derive new address.');
this.log(' $ admin [command]: Admin commands');
Expand Down
15 changes: 15 additions & 0 deletions lib/client/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,17 @@ class WalletClient extends Client {
return this.get(`/wallet/${id}/account/${account}`);
}

/**
* Get wallet account.
* @param {Number} id
* @param {Number|String} accountIndex
Copy link
Collaborator

Choose a reason for hiding this comment

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

The account index is supposed to be a Number, not a String.

* @returns {Promise}
*/

getAccountIndex(id, accountIndex) {
return this.get(`/wallet/${id}/account/id/${accountIndex}`);
}

/**
* Create account.
* @param {Number} id
Expand Down Expand Up @@ -901,6 +912,10 @@ class Wallet extends EventEmitter {
return this.client.getAccount(this.id, account);
}

getAccountIndex(account) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need JSDocs here.

return this.client.getAccountIndex(this.id, account);
}

/**
* Create account.
* @param {String} name
Expand Down
23 changes: 23 additions & 0 deletions lib/wallet/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ class HTTP extends Server {
this.get('/wallet/:id/account/:account', async (req, res) => {
const valid = Validator.fromRequest(req);
const acct = valid.str('account');

const account = await req.wallet.getAccount(acct);

if (!account) {
Expand All @@ -331,6 +332,28 @@ class HTTP extends Server {
res.json(200, account.toJSON(balance));
});

this.get('/wallet/:id/account/id/:accountIndex', async (req, res) => {
const valid = Validator.fromRequest(req);
let acct = valid.str('accountIndex');

const num = parseInt(acct);

if (!Number.isNaN(num)) {
acct = num;
}

const account = await req.wallet.getAccount(acct);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You will have to create a new method in the wallet/wallet.js file and name it getAccountByAccountIndex as there is already a method named getAccountIndex. I also ask you to name all the references of getAccountIndex to getAccountByAccountIndex.


if (!account) {
res.json(404);
return;
}

const balance = await req.wallet.getBalance(acct);

res.json(200, account.toJSON(balance));
});

// Create account
this.put('/wallet/:id/account/:account', async (req, res) => {
const valid = Validator.fromRequest(req);
Expand Down
42 changes: 38 additions & 4 deletions lib/wallet/rpc.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class RPC extends RPCBase {
this.add('getaccountaddress', this.getAccountAddress);
this.add('getaccount', this.getAccount);
this.add('getaddressesbyaccount', this.getAddressesByAccount);
this.add('getaddressesbyaccountindex', this.getAddressesByAccountIndex);
this.add('getbalance', this.getBalance);
this.add('getnewaddress', this.getNewAddress);
this.add('getrawchangeaddress', this.getRawChangeAddress);
Expand Down Expand Up @@ -443,15 +444,48 @@ class RPC extends RPCBase {

const wallet = this.wallet;
const valid = new Validator(args);
let name = valid.str(0, '');
const addrs = [];

if (name === '')
name = 'default';
let acct = valid.get(0, 'default');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem right @pinheadmz could you please take a look.


const num = parseInt(acct);
let accountName = '';

if (!Number.isNaN(num)) {
accountName = accountName + num;
acct = accountName;
}

let paths;
try {
paths = await wallet.getPaths(acct);
} catch (e) {
if (e.message === 'Account not found.')
return [];
throw e;
}

for (const path of paths) {
const addr = path.toAddress();
addrs.push(addr.toString(this.network));
}

return addrs;
}

async getAddressesByAccountIndex(args, help) {
if (help || args.length !== 1)
throw new RPCError(errs.MISC_ERROR, 'getaddressesbyaccountindex "index"');

const wallet = this.wallet;
const valid = new Validator(args);
const addrs = [];

const acct = valid.uint(0, 'default');

let paths;
try {
paths = await wallet.getPaths(name);
paths = await wallet.getPathsByIndex(acct);
} catch (e) {
if (e.message === 'Account not found.')
return [];
Expand Down
27 changes: 26 additions & 1 deletion lib/wallet/wallet.js
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,10 @@ class Wallet extends EventEmitter {
return this.wdb.getWalletPaths(this.wid);
}

async getPathsByIndex(index) {
return this.getAccountPathsByIndex(index);
}

/**
* Get all account paths.
* @param {String|Number} acct
Expand All @@ -938,7 +942,9 @@ class Wallet extends EventEmitter {
const hashes = await this.getAccountHashes(index);
const name = await this.getAccountName(acct);

assert(name);
if (name == null) {
throw new Error('Account not found.');
}

const result = [];

Expand All @@ -956,6 +962,25 @@ class Wallet extends EventEmitter {
return result;
}

async getAccountPathsByIndex(index) {
if (index === -1)
throw new Error('Account not found.');
const hashes = await this.getAccountHashes(index);

const result = [];

for (const hash of hashes) {
const path = await this.readPath(hash);

assert(path);
assert(path.account === index);

result.push(path);
}

return result;
}

/**
* Import a keyring (will not exist on derivation chain).
* Rescanning must be invoked manually.
Expand Down