Throw errors when the address is not specified or invalid

This commit is contained in:
Jacob Evans
2018-04-11 12:47:17 +10:00
parent 9169913a2c
commit 20a1deb187
9 changed files with 67 additions and 42 deletions

View File

@@ -21,7 +21,7 @@ export abstract class BaseWalletSubprovider extends Subprovider {
public abstract async getAccountsAsync(): Promise<string[]>;
public abstract async signTransactionAsync(txParams: PartialTxParams): Promise<string>;
public abstract async signPersonalMessageAsync(data: string, address?: string): Promise<string>;
public abstract async signPersonalMessageAsync(data: string, address: string): Promise<string>;
/**
* This method conforms to the web3-provider-engine interface.

View File

@@ -105,11 +105,9 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
public async signTransactionAsync(txParams: PartialTxParams): Promise<string> {
LedgerSubprovider._validateTxParams(txParams);
const initialDerivedKey = await this._initialDerivedKeyAsync();
const derivedKey = _.isUndefined(txParams.from)
? walletUtils._firstDerivedKey(initialDerivedKey)
: this._findDerivedKeyByPublicAddress(initialDerivedKey, txParams.from);
const derivedKey = this._findDerivedKeyByPublicAddress(initialDerivedKey, txParams.from);
this._ledgerClientIfExists = await this._createLedgerClientAsync();
const ledgerClient = await this._createLedgerClientAsync();
const tx = new EthereumTx(txParams);
@@ -121,7 +119,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
const txHex = tx.serialize().toString('hex');
try {
const derivationPath = `${derivedKey.derivationPath}/${derivedKey.derivationIndex}`;
const result = await this._ledgerClientIfExists.signTransaction(derivationPath, txHex);
const result = await ledgerClient.signTransaction(derivationPath, txHex);
// Store signature in transaction
tx.r = Buffer.from(result.r, 'hex');
tx.s = Buffer.from(result.s, 'hex');
@@ -145,7 +143,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
}
/**
* Sign a personal Ethereum signed message. The signing address will be the one
* either the provided address or the first address on the ledger device.
* the provided address.
* The Ledger adds the Ethereum signed message prefix on-device. If you've added
* the LedgerSubprovider to your app's provider, you can simply send an `eth_sign`
* or `personal_sign` JSON RPC request, and this method will be called auto-magically.
@@ -154,23 +152,18 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
* @param address Address to sign with
* @return Signature hex string (order: rsv)
*/
public async signPersonalMessageAsync(data: string, address?: string): Promise<string> {
public async signPersonalMessageAsync(data: string, address: string): Promise<string> {
if (_.isUndefined(data)) {
throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage);
}
assert.isHexString('data', data);
const initialDerivedKey = await this._initialDerivedKeyAsync();
const derivedKey = _.isUndefined(address)
? walletUtils._firstDerivedKey(initialDerivedKey)
: this._findDerivedKeyByPublicAddress(initialDerivedKey, address);
const derivedKey = this._findDerivedKeyByPublicAddress(initialDerivedKey, address);
this._ledgerClientIfExists = await this._createLedgerClientAsync();
const ledgerClient = await this._createLedgerClientAsync();
try {
const derivationPath = `${derivedKey.derivationPath}/${derivedKey.derivationIndex}`;
const result = await this._ledgerClientIfExists.signPersonalMessage(
derivationPath,
ethUtil.stripHexPrefix(data),
);
const result = await ledgerClient.signPersonalMessage(derivationPath, ethUtil.stripHexPrefix(data));
const v = result.v - 27;
let vHex = v.toString(16);
if (vHex.length < 2) {
@@ -192,6 +185,7 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
}
const ledgerEthereumClient = await this._ledgerEthereumClientFactoryAsync();
this._connectionLock.release();
this._ledgerClientIfExists = ledgerEthereumClient;
return ledgerEthereumClient;
}
private async _destroyLedgerClientAsync() {
@@ -205,11 +199,11 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
this._connectionLock.release();
}
private async _initialDerivedKeyAsync(): Promise<DerivedHDKey> {
this._ledgerClientIfExists = await this._createLedgerClientAsync();
const ledgerClient = await this._createLedgerClientAsync();
let ledgerResponse;
try {
ledgerResponse = await this._ledgerClientIfExists.getAddress(
ledgerResponse = await ledgerClient.getAddress(
this._derivationPath,
this._shouldAlwaysAskForConfirmation,
SHOULD_GET_CHAIN_CODE,
@@ -229,6 +223,9 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
};
}
private _findDerivedKeyByPublicAddress(initalHDKey: DerivedHDKey, address: string): DerivedHDKey {
if (_.isUndefined(address)) {
throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid);
}
const matchedDerivedKey = walletUtils.findDerivedKeyByAddress(address, initalHDKey, this._addressSearchLimit);
if (_.isUndefined(matchedDerivedKey)) {
throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`);

View File

@@ -63,6 +63,10 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider {
*/
public setPath(derivationPath: string) {
this._derivationPath = derivationPath;
this._derivedKey = {
...this._derivedKey,
derivationPath: this._derivationPath,
};
}
/**
* Retrieve the accounts associated with the mnemonic.
@@ -88,10 +92,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider {
* @return Signed transaction hex string
*/
public async signTransactionAsync(txParams: PartialTxParams): Promise<string> {
const derivedKey = _.isUndefined(txParams.from)
? walletUtils._firstDerivedKey(this._derivedKey)
: this._findDerivedKeyByPublicAddress(txParams.from);
const privateKeyWallet = new PrivateKeyWalletSubprovider(derivedKey.hdKey.privateKey.toString('hex'));
const privateKeyWallet = this._privateKeyWalletFromAddress(txParams.from);
const signedTx = privateKeyWallet.signTransactionAsync(txParams);
return signedTx;
}
@@ -105,15 +106,21 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider {
* @param address Address to sign with
* @return Signature hex string (order: rsv)
*/
public async signPersonalMessageAsync(data: string, address?: string): Promise<string> {
const derivedKey = _.isUndefined(address)
? walletUtils._firstDerivedKey(this._derivedKey)
: this._findDerivedKeyByPublicAddress(address);
const privateKeyWallet = new PrivateKeyWalletSubprovider(derivedKey.hdKey.privateKey.toString('hex'));
const sig = await privateKeyWallet.signPersonalMessageAsync(data, derivedKey.address);
public async signPersonalMessageAsync(data: string, address: string): Promise<string> {
const privateKeyWallet = this._privateKeyWalletFromAddress(address);
const sig = await privateKeyWallet.signPersonalMessageAsync(data, address);
return sig;
}
private _privateKeyWalletFromAddress(address: string): PrivateKeyWalletSubprovider {
const derivedKey = this._findDerivedKeyByPublicAddress(address);
const privateKeyHex = derivedKey.hdKey.privateKey.toString('hex');
const privateKeyWallet = new PrivateKeyWalletSubprovider(privateKeyHex);
return privateKeyWallet;
}
private _findDerivedKeyByPublicAddress(address: string): DerivedHDKey {
if (_.isUndefined(address)) {
throw new Error(WalletSubproviderErrors.FromAddressMissingOrInvalid);
}
const matchedDerivedKey = walletUtils.findDerivedKeyByAddress(
address,
this._derivedKey,

View File

@@ -63,15 +63,15 @@ export class PrivateKeyWalletSubprovider extends BaseWalletSubprovider {
* @param address Address to sign with
* @return Signature hex string (order: rsv)
*/
public async signPersonalMessageAsync(dataIfExists: string, address?: string): Promise<string> {
if (_.isUndefined(dataIfExists)) {
public async signPersonalMessageAsync(data: string, address: string): Promise<string> {
if (_.isUndefined(data)) {
throw new Error(WalletSubproviderErrors.DataMissingForSignPersonalMessage);
}
if (!_.isUndefined(address) && address !== this._address) {
throw new Error(`${WalletSubproviderErrors.AddressNotFound}: ${address}`);
if (_.isUndefined(address) || address !== this._address) {
throw new Error(`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: ${address}`);
}
assert.isHexString('data', dataIfExists);
const dataBuff = ethUtil.toBuffer(dataIfExists);
assert.isHexString('data', data);
const dataBuff = ethUtil.toBuffer(data);
const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff);
const sig = ethUtil.ecsign(msgHashBuff, this._privateKeyBuffer);
const rpcSig = ethUtil.toRpcSig(sig.v, sig.r, sig.s);

View File

@@ -80,7 +80,7 @@ export interface PartialTxParams {
gasPrice?: string;
gas: string;
to: string;
from?: string;
from: string;
value?: string;
data?: string;
chainId: number; // EIP 155 chainId - mainnet: 1, ropsten: 3
@@ -101,10 +101,10 @@ export enum WalletSubproviderErrors {
AddressNotFound = 'ADDRESS_NOT_FOUND',
DataMissingForSignPersonalMessage = 'DATA_MISSING_FOR_SIGN_PERSONAL_MESSAGE',
SenderInvalidOrNotSupplied = 'SENDER_INVALID_OR_NOT_SUPPLIED',
FromAddressMissingOrInvalid = 'FROM_ADDRESS_MISSING_OR_INVALID',
}
export enum LedgerSubproviderErrors {
TooOldLedgerFirmware = 'TOO_OLD_LEDGER_FIRMWARE',
FromAddressMissingOrInvalid = 'FROM_ADDRESS_MISSING_OR_INVALID',
MultipleOpenConnectionsDisallowed = 'MULTIPLE_OPEN_CONNECTIONS_DISALLOWED',
}

View File

@@ -55,7 +55,10 @@ describe('LedgerSubprovider', () => {
});
it('signs a personal message', async () => {
const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING));
const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync(data);
const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync(
data,
fixtureData.TEST_RPC_ACCOUNT_0,
);
expect(ecSignatureHex.length).to.be.equal(132);
expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT);
});

View File

@@ -82,7 +82,7 @@ describe('LedgerSubprovider', () => {
});
it('signs a personal message', async () => {
const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING));
const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync(data);
const ecSignatureHex = await ledgerSubprovider.signPersonalMessageAsync(data, FAKE_ADDRESS);
expect(ecSignatureHex).to.be.equal(
'0xa6cc284bff14b42bdf5e9286730c152be91719d478605ec46b3bebcd0ae491480652a1a7b742ceb0213d1e744316e285f41f878d8af0b8e632cbca4c279132d001',
);
@@ -94,7 +94,7 @@ describe('LedgerSubprovider', () => {
return expect(
Promise.all([
ledgerSubprovider.getAccountsAsync(),
ledgerSubprovider.signPersonalMessageAsync(data),
ledgerSubprovider.signPersonalMessageAsync(data, FAKE_ADDRESS),
]),
).to.be.rejectedWith(LedgerSubproviderErrors.MultipleOpenConnectionsDisallowed);
});
@@ -168,6 +168,7 @@ describe('LedgerSubprovider', () => {
gasPrice: '0x00',
nonce: '0x00',
gas: '0x00',
from: FAKE_ADDRESS,
};
const payload = {
jsonrpc: '2.0',

View File

@@ -36,7 +36,7 @@ describe('MnemonicWalletSubprovider', () => {
});
it('signs a personal message', async () => {
const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING));
const ecSignatureHex = await subprovider.signPersonalMessageAsync(data);
const ecSignatureHex = await subprovider.signPersonalMessageAsync(data, fixtureData.TEST_RPC_ACCOUNT_0);
expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT);
});
it('signs a transaction', async () => {

View File

@@ -32,7 +32,7 @@ describe('PrivateKeyWalletSubprovider', () => {
});
it('signs a personal message', async () => {
const data = ethUtils.bufferToHex(ethUtils.toBuffer(fixtureData.PERSONAL_MESSAGE_STRING));
const ecSignatureHex = await subprovider.signPersonalMessageAsync(data);
const ecSignatureHex = await subprovider.signPersonalMessageAsync(data, fixtureData.TEST_RPC_ACCOUNT_0);
expect(ecSignatureHex).to.be.equal(fixtureData.PERSONAL_MESSAGE_SIGNED_RESULT);
});
it('signs a transaction', async () => {
@@ -128,6 +128,23 @@ describe('PrivateKeyWalletSubprovider', () => {
});
provider.sendAsync(payload, callback);
});
it('should throw if `address` param is not an address from private key when calling personal_sign', (done: DoneCallback) => {
const nonHexMessage = 'hello world';
const payload = {
jsonrpc: '2.0',
method: 'personal_sign',
params: [nonHexMessage, fixtureData.TEST_RPC_ACCOUNT_1],
id: 1,
};
const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => {
expect(err).to.not.be.a('null');
expect(err.message).to.be.equal(
`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: ${fixtureData.TEST_RPC_ACCOUNT_1}`,
);
done();
});
provider.sendAsync(payload, callback);
});
it('should throw if `from` param missing when calling eth_sendTransaction', (done: DoneCallback) => {
const tx = {
to: '0xafa3f8684e54059998bc3a7b0d2b0da075154d66',
@@ -175,7 +192,7 @@ describe('PrivateKeyWalletSubprovider', () => {
};
const callback = reportCallbackErrors(done)((err: Error, response: JSONRPCResponsePayload) => {
expect(err).to.not.be.a('null');
expect(err.message).to.be.equal(`${WalletSubproviderErrors.AddressNotFound}: 0x0`);
expect(err.message).to.be.equal(`${WalletSubproviderErrors.FromAddressMissingOrInvalid}: 0x0`);
done();
});
provider.sendAsync(payload, callback);