From 85d8cf0fe021bbb8d0c43a89b9e979ffd302e700 Mon Sep 17 00:00:00 2001 From: Bagaric Date: Fri, 3 Aug 2018 12:23:17 +0200 Subject: [PATCH 1/9] Add getExecutionGasPrice function --- .../EconomicStrategyHelpers.ts | 56 +++++++++++++++++-- src/EconomicStrategy/IEconomicStrategy.ts | 25 +++++++++ src/EconomicStrategy/index.ts | 2 +- test/helpers/mockConfig.ts | 3 +- test/unit/UnitTestEconomicStrategy.ts | 43 +++++++++++++- 5 files changed, 121 insertions(+), 8 deletions(-) diff --git a/src/EconomicStrategy/EconomicStrategyHelpers.ts b/src/EconomicStrategy/EconomicStrategyHelpers.ts index 1f2f9f00..4665f50d 100644 --- a/src/EconomicStrategy/EconomicStrategyHelpers.ts +++ b/src/EconomicStrategy/EconomicStrategyHelpers.ts @@ -1,5 +1,6 @@ import { IEconomicStrategy } from './IEconomicStrategy'; import Config from '../Config'; +import { BigNumber } from '../../node_modules/bignumber.js'; /** * Checks whether a transaction requires a deposit that's higher than a @@ -7,7 +8,10 @@ import Config from '../Config'; * @param {TransactionRequest} txRequest Transaction Request object to check. * @param {IEconomicStrategy} economicStrategy Economic strategy configuration object. */ -const exceedsMaxDeposit = (txRequest: any, economicStrategy: IEconomicStrategy) => { +const exceedsMaxDeposit = ( + txRequest: any, + economicStrategy: IEconomicStrategy +): boolean | BigNumber => { const requiredDeposit = txRequest.requiredDeposit; const maxDeposit = economicStrategy.maxDeposit; @@ -22,7 +26,7 @@ const exceedsMaxDeposit = (txRequest: any, economicStrategy: IEconomicStrategy) * Checks if the balance of the TimeNode is above a set limit. * @param {Config} config TimeNode configuration object. */ -const isAboveMinBalanceLimit = async (config: Config) => { +const isAboveMinBalanceLimit = async (config: Config): Promise => { const minBalance = config.economicStrategy.minBalance; const currentBalance = await config.wallet.getBalanceOf(config.wallet.getAddresses()[0]); @@ -38,7 +42,10 @@ const isAboveMinBalanceLimit = async (config: Config) => { * @param {TransactionRequest} txRequest Transaction Request object to check. * @param {IEconomicStrategy} economicStrategy Economic strategy configuration object. */ -const isProfitable = async (txRequest: any, economicStrategy: IEconomicStrategy) => { +const isProfitable = async ( + txRequest: any, + economicStrategy: IEconomicStrategy +): Promise => { const paymentModifier = await txRequest.claimPaymentModifier(); const reward = txRequest.bounty.times(paymentModifier); @@ -51,7 +58,12 @@ const isProfitable = async (txRequest: any, economicStrategy: IEconomicStrategy) return true; }; -const shouldClaimTx = async (txRequest: any, config: Config) => { +/** + * Validates all the economic strategy parameters before claiming a certain transaction. + * @param {TransactionRequest} txRequest Transaction Request object to check. + * @param {Config} config Configuration object. + */ +const shouldClaimTx = async (txRequest: any, config: Config): Promise => { if (!config.economicStrategy) { return true; } @@ -71,4 +83,38 @@ const shouldClaimTx = async (txRequest: any, config: Config) => { return profitable && enoughBalance && !exceedsDepositLimit; }; -export { shouldClaimTx }; +/** + * Calculates the correct gas price to use for execution, taking into consideration + * the economicStrategy `maxGasSubsidy` and the current network conditions. + * @param {TransactionRequest} txRequest Transaction Request object to check. + * @param {Config} config Configuration object. + */ +const getExecutionGasPrice = async (txRequest: any, config: Config): Promise => { + const currentNetworkPrice = await config.util.networkGasPrice(); + + if (!config.economicStrategy) { + return currentNetworkPrice; + } + + const maxGasSubsidy = config.economicStrategy.maxGasSubsidy; + + if (typeof maxGasSubsidy !== 'undefined' && maxGasSubsidy !== null) { + const minGasPrice = txRequest.gasPrice; + const maxGasPrice = minGasPrice.plus(minGasPrice.times(maxGasSubsidy / 100)); + + if (currentNetworkPrice.lessThan(minGasPrice)) { + return minGasPrice; + } else if ( + currentNetworkPrice.greaterThanOrEqualTo(minGasPrice) && + currentNetworkPrice.lessThan(maxGasPrice) + ) { + return currentNetworkPrice; + } else if (currentNetworkPrice.greaterThanOrEqualTo(maxGasPrice)) { + return maxGasPrice; + } + } + + return currentNetworkPrice; +}; + +export { shouldClaimTx, getExecutionGasPrice }; diff --git a/src/EconomicStrategy/IEconomicStrategy.ts b/src/EconomicStrategy/IEconomicStrategy.ts index aa76c6fe..e49465a4 100644 --- a/src/EconomicStrategy/IEconomicStrategy.ts +++ b/src/EconomicStrategy/IEconomicStrategy.ts @@ -1,7 +1,32 @@ import BigNumber from 'bignumber.js'; export interface IEconomicStrategy { + /** + * Maximum deposit a TimeNode would be willing + * to stake while claiming a transaction. + */ maxDeposit?: BigNumber; + + /** + * Minimum balance a TimeNode has to + * have in order to claim a transaction. + */ minBalance?: BigNumber; + + /** + * Minimum profitability a scheduled transactions + * has to bring in order for the TimeNode to claim it. + */ minProfitability?: BigNumber; + + /** + * A number 0-100 which defines the percentage with which + * the TimeNode would be able to subsidize the amount of gas + * it sends at the time of the execution. + * + * e.g. If the scheduled transaction has set the gas price to 20 gwei + * and `maxGasSubsidy` is set to 50, the TimeNode would be willing + * to subsidize gas costs to up to 30 gwei. + */ + maxGasSubsidy?: number; } diff --git a/src/EconomicStrategy/index.ts b/src/EconomicStrategy/index.ts index cab5cf74..fd84366e 100644 --- a/src/EconomicStrategy/index.ts +++ b/src/EconomicStrategy/index.ts @@ -1,2 +1,2 @@ export { IEconomicStrategy } from './IEconomicStrategy'; -export { shouldClaimTx } from './EconomicStrategyHelpers'; +export { shouldClaimTx, getExecutionGasPrice } from './EconomicStrategyHelpers'; diff --git a/test/helpers/mockConfig.ts b/test/helpers/mockConfig.ts index 03ffc1cd..86b631ec 100644 --- a/test/helpers/mockConfig.ts +++ b/test/helpers/mockConfig.ts @@ -21,7 +21,8 @@ const mockConfig = (preConfig?: any) => { economicStrategy: { maxDeposit: new BigNumber(0), minBalance: new BigNumber(0), - minProfitability: new BigNumber(0) + minProfitability: new BigNumber(0), + maxGasSubsidy: 0 }, logger: new MockLogger(), ms: 4000, diff --git a/test/unit/UnitTestEconomicStrategy.ts b/test/unit/UnitTestEconomicStrategy.ts index 7dc1386e..5bf29089 100644 --- a/test/unit/UnitTestEconomicStrategy.ts +++ b/test/unit/UnitTestEconomicStrategy.ts @@ -2,7 +2,7 @@ import BigNumber from 'bignumber.js'; import { assert } from 'chai'; import { Config } from '../../src/index'; import { mockConfig, MockTxRequest } from '../helpers'; -import { shouldClaimTx } from '../../src/EconomicStrategy'; +import { shouldClaimTx, getExecutionGasPrice } from '../../src/EconomicStrategy'; describe('Economic Strategy Tests', () => { let config: Config; @@ -45,4 +45,45 @@ describe('Economic Strategy Tests', () => { assert.isFalse(shouldClaim); }); }); + + describe('getExecutionGasPrice()', () => { + it('returns current network price if economic strategy not set', async () => { + config.economicStrategy = null; + const currentNetworkPrice = await config.util.networkGasPrice(); + const gasPrice = await getExecutionGasPrice(txTimestamp, config); + assert.equal(gasPrice.toNumber(), currentNetworkPrice.toNumber()); + }); + + it('returns current network price if maxGasSubsidy not set', async () => { + config.economicStrategy.maxGasSubsidy = null; + const currentNetworkPrice = await config.util.networkGasPrice(); + const gasPrice = await getExecutionGasPrice(txTimestamp, config); + assert.equal(gasPrice.toNumber(), currentNetworkPrice.toNumber()); + }); + + it('returns tx gas price if current network price lower than tx gas price', async () => { + const gasPrice = await getExecutionGasPrice(txTimestamp, config); + assert.equal(gasPrice.toNumber(), txTimestamp.gasPrice.toNumber()); + }); + + it('returns current network price if (tx gas price + subsidy) higher than current network price', async () => { + txTimestamp.gasPrice = new BigNumber(config.web3.toWei(19, 'gwei')); + config.economicStrategy.maxGasSubsidy = 100; + const currentNetworkPrice = await config.util.networkGasPrice(); + const gasPrice = await getExecutionGasPrice(txTimestamp, config); + assert.equal(gasPrice.toNumber(), currentNetworkPrice.toNumber()); + }); + + it('returns (tx gas price + subsidy) if (tx gas price + subsidy) lower than current network price', async () => { + txTimestamp.gasPrice = new BigNumber(config.web3.toWei(19, 'gwei')); + config.economicStrategy.maxGasSubsidy = 1; + + const expectedResult = txTimestamp.gasPrice.plus( + txTimestamp.gasPrice.times(config.economicStrategy.maxGasSubsidy / 100) + ); + + const gasPrice = await getExecutionGasPrice(txTimestamp, config); + assert.equal(gasPrice.toNumber(), expectedResult.toNumber()); + }); + }); }); From 1bdbc570c9f7e0f9db70dfd6dec339126bfeb9c5 Mon Sep 17 00:00:00 2001 From: Bagaric Date: Fri, 3 Aug 2018 13:01:45 +0200 Subject: [PATCH 2/9] Fix returns --- src/EconomicStrategy/EconomicStrategyHelpers.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/EconomicStrategy/EconomicStrategyHelpers.ts b/src/EconomicStrategy/EconomicStrategyHelpers.ts index 4665f50d..33655585 100644 --- a/src/EconomicStrategy/EconomicStrategyHelpers.ts +++ b/src/EconomicStrategy/EconomicStrategyHelpers.ts @@ -1,6 +1,6 @@ import { IEconomicStrategy } from './IEconomicStrategy'; import Config from '../Config'; -import { BigNumber } from '../../node_modules/bignumber.js'; +import { BigNumber } from 'bignumber.js'; /** * Checks whether a transaction requires a deposit that's higher than a @@ -8,10 +8,7 @@ import { BigNumber } from '../../node_modules/bignumber.js'; * @param {TransactionRequest} txRequest Transaction Request object to check. * @param {IEconomicStrategy} economicStrategy Economic strategy configuration object. */ -const exceedsMaxDeposit = ( - txRequest: any, - economicStrategy: IEconomicStrategy -): boolean | BigNumber => { +const exceedsMaxDeposit = (txRequest: any, economicStrategy: IEconomicStrategy): boolean => { const requiredDeposit = txRequest.requiredDeposit; const maxDeposit = economicStrategy.maxDeposit; @@ -26,7 +23,7 @@ const exceedsMaxDeposit = ( * Checks if the balance of the TimeNode is above a set limit. * @param {Config} config TimeNode configuration object. */ -const isAboveMinBalanceLimit = async (config: Config): Promise => { +const isAboveMinBalanceLimit = async (config: Config): Promise => { const minBalance = config.economicStrategy.minBalance; const currentBalance = await config.wallet.getBalanceOf(config.wallet.getAddresses()[0]); @@ -45,7 +42,7 @@ const isAboveMinBalanceLimit = async (config: Config): Promise => { +): Promise => { const paymentModifier = await txRequest.claimPaymentModifier(); const reward = txRequest.bounty.times(paymentModifier); From adeea3c6a873f30b62666b977475a0d8b1be3385 Mon Sep 17 00:00:00 2001 From: Bagaric Date: Fri, 3 Aug 2018 13:08:10 +0200 Subject: [PATCH 3/9] Execute actions check --- src/Actions/Actions.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Actions/Actions.ts b/src/Actions/Actions.ts index 202c1b79..d15be8e1 100644 --- a/src/Actions/Actions.ts +++ b/src/Actions/Actions.ts @@ -4,6 +4,7 @@ import { isExecuted, isTransactionStatusSuccessful } from './Helpers'; import hasPending from './Pending'; import { IWalletReceipt } from '../Wallet'; import { ExecuteStatus, ClaimStatus } from '../Enum'; +import { getExecutionGasPrice } from '../EconomicStrategy'; export function shortenAddress(address: string) { return `${address.slice(0, 6)}...${address.slice(address.length - 5, address.length)}`; @@ -102,18 +103,20 @@ export default class Actions { const claimIndex = this.config.wallet.getAddresses().indexOf(txRequest.claimedBy); this.config.logger.debug(`Claim Index ${claimIndex}`); + const gasPrice = await getExecutionGasPrice(txRequest, this.config); + const opts = { to: txRequest.address, value: 0, gas: gasToExecute, - gasPrice: txRequest.gasPrice, + gasPrice, data: executeData }; if ( await hasPending(this.config, txRequest, { type: 'execute', - exactPrice: opts.gasPrice + exactPrice: gasPrice }) ) { return ExecuteStatus.PENDING; From 7266bc607b117f8fe13a411e79e4c5569c93cde3 Mon Sep 17 00:00:00 2001 From: Bagaric Date: Fri, 3 Aug 2018 13:09:54 +0200 Subject: [PATCH 4/9] Cleanup --- src/Actions/Actions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Actions/Actions.ts b/src/Actions/Actions.ts index d15be8e1..d7767a6c 100644 --- a/src/Actions/Actions.ts +++ b/src/Actions/Actions.ts @@ -116,7 +116,7 @@ export default class Actions { if ( await hasPending(this.config, txRequest, { type: 'execute', - exactPrice: gasPrice + exactPrice: opts.gasPrice }) ) { return ExecuteStatus.PENDING; From d2c20261aaf072e9a26bdf034e73b4f59be4cd47 Mon Sep 17 00:00:00 2001 From: Bagaric Date: Tue, 7 Aug 2018 12:40:17 +0200 Subject: [PATCH 5/9] Add shouldExecute check based on network gas costs --- package-lock.json | 20 +++-------- src/Actions/Actions.ts | 6 +--- .../EconomicStrategyHelpers.ts | 28 ++++++++++++++- src/EconomicStrategy/IEconomicStrategy.ts | 2 +- src/EconomicStrategy/index.ts | 2 +- src/Router/Router.ts | 34 ++++++++++++------- src/Util.ts | 9 +++++ test/e2e/TestScheduleTx.ts | 4 +-- test/helpers/mockConfig.ts | 2 +- test/unit/UnitTestEconomicStrategy.ts | 26 +++++++++++++- 10 files changed, 92 insertions(+), 41 deletions(-) diff --git a/package-lock.json b/package-lock.json index d602d604..528d9129 100644 --- a/package-lock.json +++ b/package-lock.json @@ -187,7 +187,7 @@ "integrity": "sha512-p0KlwRLui/Xn7/bXYzVhhYU9+Cv1I/JL/1CTHW1RXMz2GDFbQj0BGzohUHrv6p7QzugB6+Y/b0Z7Uh4G11RJqg==", "dev": true, "requires": { - "@types/node": "10.5.3" + "@types/node": "10.5.4" } }, "@types/lokijs": { @@ -203,9 +203,9 @@ "dev": true }, "@types/node": { - "version": "10.5.3", - "resolved": "https://registry.npmjs.org/@types/node/-/node-10.5.3.tgz", - "integrity": "sha512-jQ1p+SyF/z8ygPxfSPKZKMWazlCgTBSyIaC1UCUvkLHDdxdmPQtQFk02X4WFM31Z1BKMAS3MSAK0cRP2c92n6Q==", + "version": "10.5.4", + "resolved": "https://registry.npmjs.org/@types/node/-/node-10.5.4.tgz", + "integrity": "sha512-8TqvB0ReZWwtcd3LXq3YSrBoLyXFgBX/sBZfGye9+YS8zH7/g+i6QRIuiDmwBoTzcQ/pk89nZYTYU4c5akKkzw==", "dev": true }, "abbrev": { @@ -2344,17 +2344,6 @@ "statuses": "1.5.0" } }, - "http-proxy-agent": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/http-proxy-agent/-/http-proxy-agent-1.0.0.tgz", - "integrity": "sha1-zBzjjkU7+YSg93AtLdWcc9CBKEo=", - "dev": true, - "requires": { - "agent-base": "2.1.1", - "debug": "2.6.0", - "extend": "3.0.1" - } - }, "http-signature": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/http-signature/-/http-signature-1.2.0.tgz", @@ -6136,7 +6125,6 @@ "integrity": "sha1-uRM8VdlFdZq37mG3cRNkYg066tw=", "dev": true, "requires": { - "http-proxy-agent": "1.0.0", "https-proxy-agent": "1.0.0" } }, diff --git a/src/Actions/Actions.ts b/src/Actions/Actions.ts index 3b3375d7..396ef44a 100644 --- a/src/Actions/Actions.ts +++ b/src/Actions/Actions.ts @@ -102,11 +102,7 @@ export default class Actions { } public async execute(txRequest: any): Promise { - const gasToExecute = txRequest.callGas - .add(180000) - .div(64) - .times(65) - .round(); + const gasToExecute = this.config.util.calculateGasAmount(txRequest); // TODO Check that the gasToExecue < gasLimit of latest block w/ some margin // TODO make this a constant diff --git a/src/EconomicStrategy/EconomicStrategyHelpers.ts b/src/EconomicStrategy/EconomicStrategyHelpers.ts index 33655585..a2e89894 100644 --- a/src/EconomicStrategy/EconomicStrategyHelpers.ts +++ b/src/EconomicStrategy/EconomicStrategyHelpers.ts @@ -114,4 +114,30 @@ const getExecutionGasPrice = async (txRequest: any, config: Config): Promise => { + const isClaimedByMe = config.wallet.getAddresses().indexOf(txRequest.claimedBy) !== -1; + + const gasPrice = await this.getExecutionGasPrice(txRequest, config); + const gasAmount = config.util.calculateGasAmount(txRequest); + const reimbursement = txRequest.gasPrice.times(gasAmount); + const deposit = isClaimedByMe ? txRequest.requiredDeposit : new BigNumber(0); + + const paymentModifier = await txRequest.claimPaymentModifier(); + const reward = txRequest.bounty.times(paymentModifier); + + const gas = gasPrice.times(gasAmount); + + if (gas.greaterThan(deposit.plus(reward).plus(reimbursement))) { + return false; + } + + return true; +}; + +export { shouldClaimTx, shouldExecuteTx, getExecutionGasPrice }; diff --git a/src/EconomicStrategy/IEconomicStrategy.ts b/src/EconomicStrategy/IEconomicStrategy.ts index e49465a4..fdfe5f6a 100644 --- a/src/EconomicStrategy/IEconomicStrategy.ts +++ b/src/EconomicStrategy/IEconomicStrategy.ts @@ -20,7 +20,7 @@ export interface IEconomicStrategy { minProfitability?: BigNumber; /** - * A number 0-100 which defines the percentage with which + * A number which defines the percentage with which * the TimeNode would be able to subsidize the amount of gas * it sends at the time of the execution. * diff --git a/src/EconomicStrategy/index.ts b/src/EconomicStrategy/index.ts index fd84366e..c1f94325 100644 --- a/src/EconomicStrategy/index.ts +++ b/src/EconomicStrategy/index.ts @@ -1,2 +1,2 @@ export { IEconomicStrategy } from './IEconomicStrategy'; -export { shouldClaimTx, getExecutionGasPrice } from './EconomicStrategyHelpers'; +export { shouldClaimTx, shouldExecuteTx, getExecutionGasPrice } from './EconomicStrategyHelpers'; diff --git a/src/Router/Router.ts b/src/Router/Router.ts index 5f990d85..97ef7675 100644 --- a/src/Router/Router.ts +++ b/src/Router/Router.ts @@ -1,7 +1,7 @@ import Actions from '../Actions'; import Config from '../Config'; import { TxStatus, ClaimStatus, ExecuteStatus } from '../Enum'; -import { shouldClaimTx } from '../EconomicStrategy'; +import { shouldClaimTx, shouldExecuteTx } from '../EconomicStrategy'; import W3Util from '../Util'; import { ITxRequest } from '../Types'; @@ -110,21 +110,29 @@ export default class Router { return TxStatus.ExecutionWindow; } - try { - const executed: ExecuteStatus = await this.actions.execute(txRequest); + const shouldExecute = await shouldExecuteTx(txRequest, this.config); - if (executed === ExecuteStatus.SUCCESS) { - this.config.logger.info(`${txRequest.address} executed`); + if (shouldExecute) { + try { + const executionStatus: ExecuteStatus = await this.actions.execute(txRequest); - return TxStatus.Executed; - } else { - this.config.logger.debug(`${txRequest.address} error: ${executed}`); - } - } catch (e) { - this.config.logger.error(`${txRequest.address} execution failed`); + if (executionStatus === ExecuteStatus.SUCCESS) { + this.config.logger.info(`${txRequest.address} executed`); + + return TxStatus.Executed; + } else { + this.config.logger.debug(`${txRequest.address} error: ${executionStatus}`); + } + } catch (e) { + this.config.logger.error(`${txRequest.address} execution failed`); - //TODO handle gracefully? - throw new Error(e); + //TODO handle gracefully? + throw new Error(e); + } + } else { + this.config.logger.info( + `[${txRequest.address}] not profitable to execute. Gas price too high` + ); } return TxStatus.ExecutionWindow; diff --git a/src/Util.ts b/src/Util.ts index f5b5fb21..94da434f 100644 --- a/src/Util.ts +++ b/src/Util.ts @@ -1,4 +1,5 @@ import { IBlock } from './Types'; +import BigNumber from 'bignumber.js'; export default class W3Util { public web3: any; @@ -7,6 +8,14 @@ export default class W3Util { this.web3 = web3; } + public calculateGasAmount(txRequest: any): BigNumber { + return txRequest.callGas + .add(180000) + .div(64) + .times(65) + .round(); + } + public estimateGas(opts: any): Promise { return new Promise((resolve, reject) => { this.web3.eth.estimateGas(opts, (e: any, r: any) => { diff --git a/test/e2e/TestScheduleTx.ts b/test/e2e/TestScheduleTx.ts index 48d01903..5ef4d42e 100644 --- a/test/e2e/TestScheduleTx.ts +++ b/test/e2e/TestScheduleTx.ts @@ -73,7 +73,7 @@ export const scheduleTestTx = async () => { const { callValue } = SCHEDULED_TX_PARAMS; const callGas = new BigNumber(1000000); - const gasPrice = new BigNumber(1); + const gasPrice = new BigNumber(web3.toWei(1, 'gwei')); const fee = new BigNumber(0); const bounty = new BigNumber(0); @@ -98,7 +98,7 @@ export const scheduleTestTx = async () => { callValue, '255', // windowSize latestBlock + 270, // windowStart - 1, // gasPrice + gasPrice, // gasPrice fee, bounty, '0', // requiredDeposit diff --git a/test/helpers/mockConfig.ts b/test/helpers/mockConfig.ts index 6ec38137..2d6ad716 100644 --- a/test/helpers/mockConfig.ts +++ b/test/helpers/mockConfig.ts @@ -22,7 +22,7 @@ const mockConfig = (preConfig?: any) => { maxDeposit: new BigNumber(0), minBalance: new BigNumber(0), minProfitability: new BigNumber(0), - maxGasSubsidy: 0 + maxGasSubsidy: 100 }, logger: new MockLogger(), ms: 4000, diff --git a/test/unit/UnitTestEconomicStrategy.ts b/test/unit/UnitTestEconomicStrategy.ts index 5bf29089..750fb959 100644 --- a/test/unit/UnitTestEconomicStrategy.ts +++ b/test/unit/UnitTestEconomicStrategy.ts @@ -2,7 +2,7 @@ import BigNumber from 'bignumber.js'; import { assert } from 'chai'; import { Config } from '../../src/index'; import { mockConfig, MockTxRequest } from '../helpers'; -import { shouldClaimTx, getExecutionGasPrice } from '../../src/EconomicStrategy'; +import { shouldClaimTx, getExecutionGasPrice, shouldExecuteTx } from '../../src/EconomicStrategy'; describe('Economic Strategy Tests', () => { let config: Config; @@ -62,6 +62,7 @@ describe('Economic Strategy Tests', () => { }); it('returns tx gas price if current network price lower than tx gas price', async () => { + config.economicStrategy.maxGasSubsidy = 100; const gasPrice = await getExecutionGasPrice(txTimestamp, config); assert.equal(gasPrice.toNumber(), txTimestamp.gasPrice.toNumber()); }); @@ -86,4 +87,27 @@ describe('Economic Strategy Tests', () => { assert.equal(gasPrice.toNumber(), expectedResult.toNumber()); }); }); + + describe('shouldExecuteTx()', () => { + it('returns true if CurrentGasCost < (Deposit + Reward + Reimbursement)', async () => { + txTimestamp.claimedBy = config.wallet.getAddresses()[0]; + const shouldExecute = await shouldExecuteTx(txTimestamp, config); + assert.isTrue(shouldExecute); + }); + + it('returns false if CurrentGasCost > (Deposit + Reward + Reimbursement)', async () => { + txTimestamp.claimedBy = config.wallet.getAddresses()[0]; + txTimestamp.requiredDeposit = new BigNumber(config.web3.toWei(0, 'ether')); + txTimestamp.bounty = new BigNumber(config.web3.toWei(0.1, 'gwei')); + config.util.networkGasPrice = () => new BigNumber(config.web3.toWei(100, 'gwei')); + + const shouldExecute = await shouldExecuteTx(txTimestamp, config); + assert.isFalse(shouldExecute); + }); + + it('returns true if CurrentGasCost < (Reward + Reimbursement) and not claimed by me', async () => { + const shouldExecute = await shouldExecuteTx(txTimestamp, config); + assert.isTrue(shouldExecute); + }); + }); }); From 01ad2c90877e577706f39168a0a59d7aedb7796d Mon Sep 17 00:00:00 2001 From: Bagaric Date: Tue, 7 Aug 2018 13:02:44 +0200 Subject: [PATCH 6/9] Raise gas price for test tx --- test/e2e/TestScheduleTx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/TestScheduleTx.ts b/test/e2e/TestScheduleTx.ts index 5ef4d42e..f88382d1 100644 --- a/test/e2e/TestScheduleTx.ts +++ b/test/e2e/TestScheduleTx.ts @@ -73,7 +73,7 @@ export const scheduleTestTx = async () => { const { callValue } = SCHEDULED_TX_PARAMS; const callGas = new BigNumber(1000000); - const gasPrice = new BigNumber(web3.toWei(1, 'gwei')); + const gasPrice = new BigNumber(web3.toWei(20, 'gwei')); const fee = new BigNumber(0); const bounty = new BigNumber(0); From a018a65738b0902fc00df773b8a6151936ec70a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kosi=C5=84ski?= Date: Wed, 8 Aug 2018 09:48:23 +0200 Subject: [PATCH 7/9] Fix logging scheme to match expected one in the test suite --- src/Router/Router.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Router/Router.ts b/src/Router/Router.ts index efb5c72a..cfbbae00 100644 --- a/src/Router/Router.ts +++ b/src/Router/Router.ts @@ -117,14 +117,14 @@ export default class Router { const executionStatus: ExecuteStatus = await this.actions.execute(txRequest); if (executionStatus === ExecuteStatus.SUCCESS) { - this.config.logger.info(`${txRequest.address} executed`); + this.config.logger.info(`[${txRequest.address}] executed`); return TxStatus.Executed; } else { - this.config.logger.debug(`${txRequest.address} error: ${executionStatus}`); + this.config.logger.debug(`[${txRequest.address}] error: ${executionStatus}`); } } catch (e) { - this.config.logger.error(`${txRequest.address} execution failed`); + this.config.logger.error(`[${txRequest.address}] execution failed`); //TODO handle gracefully? throw new Error(e); From 1550f35b435af07e20eb2d92af0cb3bf4f5d1e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kosi=C5=84ski?= Date: Wed, 8 Aug 2018 10:10:58 +0200 Subject: [PATCH 8/9] Added logging to shouldExecuteTx --- src/EconomicStrategy/EconomicStrategyHelpers.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/EconomicStrategy/EconomicStrategyHelpers.ts b/src/EconomicStrategy/EconomicStrategyHelpers.ts index 11bd62e6..26b3dc4f 100644 --- a/src/EconomicStrategy/EconomicStrategyHelpers.ts +++ b/src/EconomicStrategy/EconomicStrategyHelpers.ts @@ -132,12 +132,16 @@ const shouldExecuteTx = async (txRequest: any, config: Config): Promise const reward = txRequest.bounty.times(paymentModifier); const gasCost = gasPrice.times(gasAmount); + const expectedReward = deposit.plus(reward).plus(reimbursement); + const shouldExecuteTx = gasCost.lessThanOrEqualTo(expectedReward); - if (gasCost.greaterThan(deposit.plus(reward).plus(reimbursement))) { - return false; - } + config.logger.debug( + `[${ + txRequest.address + }] shouldExecuteTx ret ${shouldExecuteTx} gasCost=${gasCost.toNumber()} expectedReward=${expectedReward.toNumber()}` + ); - return true; + return shouldExecuteTx; }; export { shouldClaimTx, shouldExecuteTx, getExecutionGasPrice }; From e65b4ef87e3550b61c4fc2abe1d26886afe7b346 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kosi=C5=84ski?= Date: Wed, 8 Aug 2018 10:17:31 +0200 Subject: [PATCH 9/9] lint fix --- src/EconomicStrategy/EconomicStrategyHelpers.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/EconomicStrategy/EconomicStrategyHelpers.ts b/src/EconomicStrategy/EconomicStrategyHelpers.ts index 26b3dc4f..1f72c4fc 100644 --- a/src/EconomicStrategy/EconomicStrategyHelpers.ts +++ b/src/EconomicStrategy/EconomicStrategyHelpers.ts @@ -133,15 +133,15 @@ const shouldExecuteTx = async (txRequest: any, config: Config): Promise const gasCost = gasPrice.times(gasAmount); const expectedReward = deposit.plus(reward).plus(reimbursement); - const shouldExecuteTx = gasCost.lessThanOrEqualTo(expectedReward); + const shouldExecute = gasCost.lessThanOrEqualTo(expectedReward); config.logger.debug( `[${ txRequest.address - }] shouldExecuteTx ret ${shouldExecuteTx} gasCost=${gasCost.toNumber()} expectedReward=${expectedReward.toNumber()}` + }] shouldExecuteTx ret ${shouldExecute} gasCost=${gasCost.toNumber()} expectedReward=${expectedReward.toNumber()}` ); - return shouldExecuteTx; + return shouldExecute; }; export { shouldClaimTx, shouldExecuteTx, getExecutionGasPrice };