Skip to content

Commit

Permalink
Fixed and refactored populating transaction values for signers (#306).
Browse files Browse the repository at this point in the history
  • Loading branch information
@ricmoo
ricmoo committed Oct 14, 2018
1 parent e39cd84 commit 023a20f
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 87 deletions.
58 changes: 23 additions & 35 deletions src.ts/providers/json-rpc-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,23 @@ import { BaseProvider } from './base-provider';

import { Signer } from '../abstract-signer';

import * as errors from '../errors';

import { getAddress } from '../utils/address';
import { BigNumber } from '../utils/bignumber';
import { hexlify, hexStripZeros } from '../utils/bytes';
import { getNetwork } from '../utils/networks';
import { defineReadOnly, resolveProperties, shallowCopy } from '../utils/properties';
import { checkProperties, defineReadOnly, shallowCopy } from '../utils/properties';
import { populateTransaction } from '../utils/transaction';
import { toUtf8Bytes } from '../utils/utf8';
import { fetchJson, poll } from '../utils/web';

// Imported Types
import { Arrayish } from '../utils/bytes';
import { Network, Networkish } from '../utils/networks';
import { ConnectionInfo } from '../utils/web';
import { BlockTag, TransactionRequest, TransactionResponse } from '../providers/abstract-provider';

import * as errors from '../errors';
import { BlockTag, TransactionRequest, TransactionResponse } from '../providers/abstract-provider';

function timer(timeout: number): Promise<any> {
return new Promise(function(resolve) {
Expand Down Expand Up @@ -78,15 +80,6 @@ export class JsonRpcSigner extends Signer {
}
}

/* May add back in the future; for now it is considered confusing. :)
get address(): string {
if (!this._address) {
errors.throwError('no sync sync address available; use getAddress', errors.UNSUPPORTED_OPERATION, { operation: 'address' });
}
return this._address
}
*/

getAddress(): Promise<string> {
if (this._address) {
return Promise.resolve(this._address);
Expand All @@ -110,21 +103,18 @@ export class JsonRpcSigner extends Signer {
}

sendTransaction(transaction: TransactionRequest): Promise<TransactionResponse> {
let tx: TransactionRequest = shallowCopy(transaction);

if (tx.from == null) {
tx.from = this.getAddress().then((address) => {
if (!address) { return null; }
return address.toLowerCase();
});
}

if (transaction.gasLimit == null) {
tx.gasLimit = this.provider.estimateGas(tx);
}
// Once populateTransaction resolves, the from address will be populated from getAddress
let from: string = null;
let getAddress = this.getAddress().then((address) => {
if (address) { from = address.toLowerCase(); }
return from;
});

return resolveProperties(tx).then((tx) => {
return this.provider.send('eth_sendTransaction', [ JsonRpcProvider.hexlifyTransaction(tx) ]).then((hash) => {
return populateTransaction(transaction, this.provider, getAddress).then((tx) => {
let hexTx = JsonRpcProvider.hexlifyTransaction(tx);
hexTx.from = from;
return this.provider.send('eth_sendTransaction', [ hexTx ]).then((hash) => {
return poll(() => {
return this.provider.getTransaction(hash).then((tx: TransactionResponse) => {
if (tx === null) { return undefined; }
Expand Down Expand Up @@ -376,21 +366,19 @@ export class JsonRpcProvider extends BaseProvider {
// NOTE: This allows a TransactionRequest, but all values should be resolved
// before this is called
static hexlifyTransaction(transaction: TransactionRequest, allowExtra?: { [key: string]: boolean }): { [key: string]: string } {
if (!allowExtra) { allowExtra = {}; }

for (let key in transaction) {
if (!allowedTransactionKeys[key] && !allowExtra[key]) {
errors.throwError('invalid key - ' + key, errors.INVALID_ARGUMENT, {
argument: 'transaction',
value: transaction,
key: key
});

// Check only allowed properties are given
let allowed = shallowCopy(allowedTransactionKeys);
if (allowExtra) {
for (let key in allowExtra) {
if (allowExtra[key]) { allowed[key] = true; }
}
}
checkProperties(transaction, allowed);

let result: { [key: string]: string } = {};

// Some nodes (INFURA ropsten; INFURA mainnet is fine) don't like extra zeros.
// Some nodes (INFURA ropsten; INFURA mainnet is fine) don't like leading zeros.
['gasLimit', 'gasPrice', 'nonce', 'value'].forEach(function(key) {
if ((<any>transaction)[key] == null) { return; }
let value = hexStripZeros(hexlify((<any>transaction)[key]));
Expand Down
5 changes: 4 additions & 1 deletion src.ts/utils/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ import { sha256 } from './sha2';
import { keccak256 as solidityKeccak256, pack as solidityPack, sha256 as soliditySha256 } from './solidity';
import { randomBytes } from './random-bytes';
import { getNetwork } from './networks';
import { deepCopy, defineReadOnly, resolveProperties, shallowCopy } from './properties';
import { checkProperties, deepCopy, defineReadOnly, resolveProperties, shallowCopy } from './properties';
import * as RLP from './rlp';
import { computeAddress, computePublicKey, recoverAddress, recoverPublicKey, verifyMessage } from './secp256k1';
import { SigningKey } from './signing-key';
import { populateTransaction } from './transaction';
import { parse as parseTransaction, serialize as serializeTransaction } from './transaction';
import { formatBytes32String, parseBytes32String, toUtf8Bytes, toUtf8String } from './utf8';
import { commify, formatEther, parseEther, formatUnits, parseUnits } from './units';
Expand Down Expand Up @@ -60,6 +61,7 @@ export {
fetchJson,
getNetwork,

checkProperties,
deepCopy,
defineReadOnly,
resolveProperties,
Expand Down Expand Up @@ -122,6 +124,7 @@ export {
joinSignature,

parseTransaction,
populateTransaction,
serializeTransaction,

getJsonWalletAddress,
Expand Down
21 changes: 21 additions & 0 deletions src.ts/utils/properties.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

import * as errors from '../errors';

export function defineReadOnly(object: any, name: string, value: any): void {
Object.defineProperty(object, name, {
enumerable: true,
Expand Down Expand Up @@ -42,6 +44,25 @@ export function resolveProperties(object: any): Promise<any> {
});
}

export function checkProperties(object: any, properties: { [ name: string ]: boolean }): void {
if (!object || typeof(object) !== 'object') {
errors.throwError('invalid object', errors.INVALID_ARGUMENT, {
argument: 'object',
value: object
});
}

Object.keys(object).forEach((key) => {
if (!properties[key]) {
errors.throwError('invalid object key - ' + key, errors.INVALID_ARGUMENT, {
argument: 'transaction',
value: object,
key: key
});
}
});
}

export function shallowCopy(object: any): any {
let result: any = {};
for (var key in object) { result[key] = object[key]; }
Expand Down
57 changes: 52 additions & 5 deletions src.ts/utils/transaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { getAddress } from './address';
import { BigNumber, bigNumberify } from './bignumber';
import { arrayify, hexlify, hexZeroPad, splitSignature, stripZeros, } from './bytes';
import { keccak256 } from './keccak256';
import { checkProperties, resolveProperties, shallowCopy } from './properties';

import * as RLP from './rlp';

Expand All @@ -19,6 +20,8 @@ import * as RLP from './rlp';
import { Arrayish, Signature } from './bytes';
import { BigNumberish } from './bignumber';

import { Provider } from '../providers/abstract-provider';

///////////////////////////////
// Exported Types

Expand Down Expand Up @@ -65,7 +68,7 @@ function handleNumber(value: string): BigNumber {
return bigNumberify(value);
}

var transactionFields = [
const transactionFields = [
{ name: 'nonce', maxLength: 32 },
{ name: 'gasPrice', maxLength: 32 },
{ name: 'gasLimit', maxLength: 32 },
Expand All @@ -74,8 +77,14 @@ var transactionFields = [
{ name: 'data' },
];

const allowedTransactionKeys: { [ key: string ]: boolean } = {
chainId: true, data: true, gasLimit: true, gasPrice:true, nonce: true, to: true, value: true
}

export function serialize(transaction: UnsignedTransaction, signature?: Arrayish | Signature): string {
var raw: Array<string | Uint8Array> = [];
checkProperties(transaction, allowedTransactionKeys);

let raw: Array<string | Uint8Array> = [];

transactionFields.forEach(function(fieldInfo) {
let value = (<any>transaction)[fieldInfo.name] || ([]);
Expand Down Expand Up @@ -115,7 +124,7 @@ export function serialize(transaction: UnsignedTransaction, signature?: Arrayish
let sig = splitSignature(signature);

// We pushed a chainId and null r, s on for hashing only; remove those
var v = 27 + sig.recoveryParam
let v = 27 + sig.recoveryParam
if (raw.length === 9) {
raw.pop();
raw.pop();
Expand Down Expand Up @@ -171,7 +180,7 @@ export function parse(rawTransaction: Arrayish): Transaction {
tx.chainId = Math.floor((tx.v - 35) / 2);
if (tx.chainId < 0) { tx.chainId = 0; }

var recoveryParam = tx.v - 27;
let recoveryParam = tx.v - 27;

let raw = transaction.slice(0, 6);

Expand All @@ -182,7 +191,7 @@ export function parse(rawTransaction: Arrayish): Transaction {
recoveryParam -= tx.chainId * 2 + 8;
}

var digest = keccak256(RLP.encode(raw));
let digest = keccak256(RLP.encode(raw));
try {
tx.from = recoverAddress(digest, { r: hexlify(tx.r), s: hexlify(tx.s), recoveryParam: recoveryParam });
} catch (error) {
Expand All @@ -194,3 +203,41 @@ export function parse(rawTransaction: Arrayish): Transaction {

return tx;
}

export function populateTransaction(transaction: any, provider: Provider, from: string | Promise<string>): Promise<Transaction> {

if (!Provider.isProvider(provider)) {
errors.throwError('missing provider', errors.INVALID_ARGUMENT, {
argument: 'provider',
value: provider
});
}

checkProperties(transaction, allowedTransactionKeys);

let tx = shallowCopy(transaction);

if (tx.to != null) {
tx.to = provider.resolveName(tx.to);
}

if (tx.gasPrice == null) {
tx.gasPrice = provider.getGasPrice();
}

if (tx.nonce == null) {
tx.nonce = provider.getTransactionCount(from);
}

if (tx.gasLimit == null) {
let estimate = shallowCopy(tx);
estimate.from = from;
tx.gasLimit = provider.estimateGas(estimate);
}

if (tx.chainId == null) {
tx.chainId = provider.getNetwork().then((network) => network.chainId);
}

return resolveProperties(tx);
}
53 changes: 7 additions & 46 deletions src.ts/wallet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { defineReadOnly, resolveProperties, shallowCopy } from './utils/properti
import { randomBytes } from './utils/random-bytes';
import * as secretStorage from './utils/secret-storage';
import { SigningKey } from './utils/signing-key';
import { serialize as serializeTransaction } from './utils/transaction';
import { populateTransaction, serialize as serializeTransaction } from './utils/transaction';
import { Wordlist } from './utils/wordlist';

// Imported Abstracts
Expand All @@ -24,10 +24,6 @@ import { BlockTag, TransactionRequest, TransactionResponse } from './providers/a

import * as errors from './errors';

const allowedTransactionKeys: { [ key: string ]: boolean } = {
chainId: true, data: true, gasLimit: true, gasPrice:true, nonce: true, to: true, value: true
}

export class Wallet extends AbstractSigner {

readonly provider: Provider;
Expand Down Expand Up @@ -72,19 +68,10 @@ export class Wallet extends AbstractSigner {
}

sign(transaction: TransactionRequest): Promise<string> {
for (let key in transaction) {
if (!allowedTransactionKeys[key]) {
errors.throwError('unsupported transaction property - ' + key, errors.INVALID_ARGUMENT, {
argument: 'transaction',
value: transaction,
key: key
});
}
}
return resolveProperties(transaction).then((tx) => {
let rawTx = serializeTransaction(tx);
let signature = this.signingKey.signDigest(keccak256(rawTx));
return Promise.resolve(serializeTransaction(tx, signature));
return serializeTransaction(tx, signature);
});
}

Expand All @@ -104,37 +91,11 @@ export class Wallet extends AbstractSigner {
}

sendTransaction(transaction: TransactionRequest): Promise<TransactionResponse> {
if (!this.provider) { throw new Error('missing provider'); }

if (!transaction || typeof(transaction) !== 'object') {
throw new Error('invalid transaction object');
}

let tx = shallowCopy(transaction);

if (tx.to != null) {
tx.to = this.provider.resolveName(tx.to);
}

if (tx.gasPrice == null) {
tx.gasPrice = this.provider.getGasPrice();
}

if (tx.nonce == null) {
tx.nonce = this.getTransactionCount();
}

if (tx.gasLimit == null) {
let estimate = shallowCopy(tx);
estimate.from = this.getAddress();
tx.gasLimit = this.provider.estimateGas(estimate);
}

if (tx.chainId == null) {
tx.chainId = this.provider.getNetwork().then((network) => network.chainId);
}

return this.provider.sendTransaction(this.sign(tx));
return populateTransaction(transaction, this.provider, this.address).then((tx) => {
return this.sign(tx).then((signedTransaction) => {
return this.provider.sendTransaction(signedTransaction);
});
});
}

encrypt(password: Arrayish | string, options?: any, progressCallback?: ProgressCallback): Promise<string> {
Expand Down

0 comments on commit 023a20f

Please sign in to comment.