[ck] consistent send tx object in kit (#1191)

packages/contractkit/src/kit.test.ts

@@ -0,0 +1,84 @@
+import { TransactionObject, Tx } from 'web3/eth/types'
+import PromiEvent from 'web3/promiEvent'
+import { TransactionReceipt } from 'web3/types'
+import { newKit } from './kit'
+import { promiEventSpy } from './test-utils/PromiEventStub'
+
+interface TransactionObjectStub<T> extends TransactionObject<T> {
+  sendMock: jest.Mock<PromiEvent<any>, [Tx | undefined]>
+  estimateGasMock: jest.Mock<Promise<number>, []>
+  resolveHash(hash: string): void
+  resolveReceipt(receipt: TransactionReceipt): void
+  rejectHash(error: any): void
+  rejectReceipt(receipt: TransactionReceipt, error: any): void
+}
+
+export function txoStub<T>(): TransactionObjectStub<T> {
+  const estimateGasMock = jest.fn()
+  const peStub = promiEventSpy()
+  const sendMock = jest.fn().mockReturnValue(peStub)
+
+  const pe: TransactionObjectStub<T> = {
+    arguments: [],
+    call: () => {
+      throw new Error('not implemented')
+    },
+    encodeABI: () => {
+      throw new Error('not implemented')
+    },
+    estimateGas: estimateGasMock,
+    send: sendMock,
+    sendMock,
+    estimateGasMock,
+    resolveHash: peStub.resolveHash,
+    rejectHash: peStub.rejectHash,
+    resolveReceipt: peStub.resolveReceipt,
+    rejectReceipt: peStub.resolveReceipt,
+  }
+  return pe
+}
+
+describe('kit.sendTransactionObject()', () => {
+  const kit = newKit('http://')
+
+  test('should send transaction on simple case', async () => {
+    const txo = txoStub()
+    txo.estimateGasMock.mockResolvedValue(1000)
+    const txRes = await kit.sendTransactionObject(txo)
+
+    txo.resolveHash('HASH')
+    txo.resolveReceipt('Receipt' as any)
+
+    await expect(txRes.getHash()).resolves.toBe('HASH')
+    await expect(txRes.waitReceipt()).resolves.toBe('Receipt')
+  })
+
+  test('should not estimateGas if gas is provided', async () => {
+    const txo = txoStub()
+    await kit.sendTransactionObject(txo, { gas: 555 })
+    expect(txo.estimateGasMock).not.toBeCalled()
+  })
+
+  test('should use inflation factor on gas', async () => {
+    const txo = txoStub()
+    txo.estimateGasMock.mockResolvedValue(1000)
+    kit.gasInflactionFactor = 2
+    await kit.sendTransactionObject(txo)
+    expect(txo.send).toBeCalledWith(
+      expect.objectContaining({
+        gas: 1000 * 2,
+      })
+    )
+  })
+
+  test('should forward txoptions to txo.send()', async () => {
+    const txo = txoStub()
+    await kit.sendTransactionObject(txo, { gas: 555, gasCurrency: 'XXX', from: '0xAAFFF' })
+    expect(txo.send).toBeCalledWith({
+      gasPrice: '0',
+      gas: 555,
+      gasCurrency: 'XXX',
+      from: '0xAAFFF',
+    })
+  })
+})

packages/contractkit/src/kit.ts

@@ -1,9 +1,9 @@
+import debugFactory from 'debug'
 import Web3 from 'web3'
 import { TransactionObject, Tx } from 'web3/eth/types'
 import { AddressRegistry } from './address-registry'
 import { Address, CeloContract, CeloToken } from './base'
 import { WrapperCache } from './contract-cache'
-import { sendTransaction, TxOptions } from './utils/send-tx'
 import { toTxResult, TransactionResult } from './utils/tx-result'
 import { addLocalAccount } from './utils/web3-utils'
 import { Web3ContractCache } from './web3-contract-cache'
@@ -17,6 +17,8 @@ import { SortedOraclesConfig } from './wrappers/SortedOracles'
 import { StableTokenConfig } from './wrappers/StableTokenWrapper'
 import { ValidatorConfig } from './wrappers/Validators'
 
+const debug = debugFactory('kit:kit')
+
 export function newKit(url: string) {
   return newKitFromWeb3(new Web3(url))
 }
@@ -37,14 +39,21 @@ export interface NetworkConfig {
   validators: ValidatorConfig
 }
 
+export interface KitOptions {
+  gasInflationFactor: number
+  gasCurrency: Address | null
+  from?: Address
+}
+
 export class ContractKit {
   readonly registry: AddressRegistry
   readonly _web3Contracts: Web3ContractCache
   readonly contracts: WrapperCache
 
-  private _defaultOptions: TxOptions
+  private config: KitOptions
   constructor(readonly web3: Web3) {
-    this._defaultOptions = {
+    this.config = {
+      gasCurrency: null,
       gasInflationFactor: 1.3,
     }
 
@@ -92,29 +101,37 @@ export class ContractKit {
   }
 
   async setGasCurrency(token: CeloToken) {
-    this._defaultOptions.gasCurrency =
-      token === CeloContract.GoldToken ? undefined : await this.registry.addressFor(token)
+    this.config.gasCurrency =
+      token === CeloContract.GoldToken ? null : await this.registry.addressFor(token)
   }
 
   addAccount(privateKey: string) {
     addLocalAccount(this.web3, privateKey)
   }
 
   set defaultAccount(address: Address) {
-    this._defaultOptions.from = address
+    this.config.from = address
     this.web3.eth.defaultAccount = address
   }
 
   get defaultAccount(): Address {
     return this.web3.eth.defaultAccount
   }
 
-  get defaultOptions(): Readonly<TxOptions> {
-    return { ...this._defaultOptions }
+  set gasInflactionFactor(factor: number) {
+    this.config.gasInflationFactor = factor
+  }
+
+  get gasInflationFactor() {
+    return this.config.gasInflationFactor
+  }
+
+  set defaultGasCurrency(address: Address | null) {
+    this.config.gasCurrency = address
   }
 
-  setGasCurrencyAddress(address: Address) {
-    this._defaultOptions.gasCurrency = address
+  get defaultGasCurrency() {
+    return this.config.gasCurrency
   }
 
   isListening(): Promise<boolean> {
@@ -125,27 +142,59 @@ export class ContractKit {
     return this.web3.eth.isSyncing()
   }
 
-  sendTransaction(tx: Tx): TransactionResult {
-    const promiEvent = this.web3.eth.sendTransaction({
-      from: this._defaultOptions.from,
-      // TODO this won't work for locally signed TX
-      gasPrice: '0',
-      // @ts-ignore
-      gasCurrency: this._defaultOptions.gasCurrency,
-      // TODO needed for locally signed tx, ignored by now (celo-blockchain with set it)
-      // gasFeeRecipient: this.defaultOptions.gasFeeRecipient,
-      ...tx,
-    })
-    return toTxResult(promiEvent)
+  async sendTransaction(tx: Tx): Promise<TransactionResult> {
+    tx = this.fillTxDefaults(tx)
+
+    let gas = tx.gas
+    if (gas == null) {
+      gas = Math.round(
+        (await this.web3.eth.estimateGas({ ...tx })) * this.config.gasInflationFactor
+      )
+      debug('estimatedGas: %s', gas)
+    }
+
+    return toTxResult(
+      this.web3.eth.sendTransaction({
+        ...tx,
+        gas,
+      })
+    )
   }
 
-  sendTransactionObject(
+  async sendTransactionObject(
     txObj: TransactionObject<any>,
-    options?: TxOptions
+    tx?: Omit<Tx, 'data'>
   ): Promise<TransactionResult> {
-    return sendTransaction(txObj, {
-      ...this._defaultOptions,
-      ...options,
-    })
+    tx = this.fillTxDefaults(tx)
+
+    let gas = tx.gas
+    if (gas == null) {
+      gas = Math.round((await txObj.estimateGas({ ...tx })) * this.config.gasInflationFactor)
+      debug('estimatedGas: %s', gas)
+    }
+
+    return toTxResult(
+      txObj.send({
+        ...tx,
+        gas,
+      })
+    )
+  }
+
+  private fillTxDefaults(tx?: Tx): Tx {
+    const defaultTx: Tx = {
+      from: this.config.from,
+      // gasPrice:0 means the node will compute gasPrice on it's own
+      gasPrice: '0',
+    }
+
+    if (this.config.gasCurrency) {
+      defaultTx.gasCurrency = this.config.gasCurrency
+    }
+
+    return {
+      ...defaultTx,
+      ...tx,
+    }
   }
 }