Feature #322: Set safeTxGas for newly created transactions (#752)

* Set safeTxGas for newly created transactions * extending estimation for nested calls WIP * messing up things to implement safeTxGas estimation * simplifying base gas calculations * remove eslint disable * messing things up to make things work * bring estimateData back * pass payload to requiredTxGas * pass esimateData to estimateDataGasCosts * remove console log * use batch requests to estimate safeTxGas * use const for txGasEstimation * check the response in request simultion * sidebar fix * remove for loop
2020-04-20 14:54:09 +04:00 · 2020-04-20 14:54:09 +04:00 · 204edd635d
parent 2e5d1564a2
commit 204edd635d
7 changed files with 1024 additions and 503 deletions
--- a/package.json
+++ b/package.json
@ -43,37 +43,37 @@
  "dependencies": {
    "@gnosis.pm/safe-contracts": "1.1.1-dev.1",
    "@gnosis.pm/util-contracts": "2.0.6",
-    "@material-ui/core": "4.9.8",
+    "@material-ui/core": "4.9.10",
    "@material-ui/icons": "4.9.1",
    "@material-ui/lab": "4.0.0-alpha.39",
    "@openzeppelin/contracts": "^2.5.0",
-    "@testing-library/jest-dom": "5.3.0",
-    "@welldone-software/why-did-you-render": "4.0.5",
+    "@testing-library/jest-dom": "5.5.0",
+    "@welldone-software/why-did-you-render": "4.0.8",
    "async-sema": "^3.1.0",
    "axios": "0.19.2",
    "bignumber.js": "9.0.0",
-    "bnc-onboard": "^1.7.1",
+    "bnc-onboard": "1.7.5",
    "connected-react-router": "6.8.0",
    "currency-flags": "^2.1.1",
-    "date-fns": "2.11.1",
+    "date-fns": "2.12.0",
    "dotenv": "^8.2.0",
    "ethereum-ens": "0.8.0",
-    "final-form": "4.18.7",
+    "final-form": "4.19.1",
    "history": "4.10.1",
    "immortal-db": "^1.0.2",
    "immutable": "^4.0.0-rc.9",
    "js-cookie": "^2.2.1",
-    "lint-staged": "10.0.10",
+    "lint-staged": "10.1.3",
    "material-ui-search-bar": "^1.0.0-beta.13",
    "notistack": "https://github.com/gnosis/notistack.git#v0.9.4",
    "optimize-css-assets-webpack-plugin": "5.0.3",
    "polished": "3.5.1",
    "qrcode.react": "1.0.0",
-    "query-string": "6.11.1",
+    "query-string": "6.12.1",
    "react": "16.13.1",
    "react-dev-utils": "10.2.1",
    "react-dom": "16.13.1",
-    "react-final-form": "6.3.5",
+    "react-final-form": "6.4.0",
    "react-final-form-listeners": "^1.0.2",
    "react-ga": "^2.7.0",
    "react-hot-loader": "4.12.20",
@ -86,7 +86,7 @@
    "redux-actions": "^2.6.5",
    "redux-thunk": "^2.3.0",
    "reselect": "^4.0.0",
-    "semver": "^7.1.1",
+    "semver": "7.3.2",
    "styled-components": "^5.0.1",
    "web3": "1.2.6"
  },
@ -112,21 +112,21 @@
    "@babel/plugin-transform-member-expression-literals": "7.8.3",
    "@babel/plugin-transform-property-literals": "7.8.3",
    "@babel/polyfill": "7.8.7",
-    "@babel/preset-env": "7.9.0",
+    "@babel/preset-env": "7.9.5",
    "@babel/preset-flow": "7.9.0",
    "@babel/preset-react": "7.9.4",
-    "@testing-library/react": "10.0.1",
-    "autoprefixer": "9.7.5",
+    "@testing-library/react": "10.0.2",
+    "autoprefixer": "9.7.6",
    "babel-core": "^7.0.0-bridge.0",
    "babel-eslint": "10.1.0",
-    "babel-jest": "25.2.4",
+    "babel-jest": "25.3.0",
    "babel-loader": "8.1.0",
    "babel-plugin-dynamic-import-node": "^2.3.0",
    "babel-plugin-transform-es3-member-expression-literals": "^6.22.0",
    "babel-plugin-transform-es3-property-literals": "^6.22.0",
    "babel-polyfill": "^6.26.0",
    "classnames": "^2.2.6",
-    "css-loader": "3.4.2",
+    "css-loader": "3.5.2",
    "detect-port": "^1.3.0",
    "dotenv-expand": "^5.1.0",
    "eslint": "^6.8.0",
@ -140,28 +140,28 @@
    "ethereumjs-abi": "0.6.8",
    "extract-text-webpack-plugin": "^4.0.0-beta.0",
    "file-loader": "6.0.0",
-    "flow-bin": "0.121.0",
+    "flow-bin": "0.122.0",
    "fs-extra": "9.0.0",
-    "html-loader": "1.0.0",
-    "html-webpack-plugin": "4.0.3",
+    "html-loader": "1.1.0",
+    "html-webpack-plugin": "4.2.0",
    "husky": "^4.2.2",
-    "jest": "25.2.4",
+    "jest": "25.3.0",
    "jest-dom": "4.0.0",
    "json-loader": "^0.5.7",
    "mini-css-extract-plugin": "0.9.0",
    "postcss-loader": "^3.0.0",
    "postcss-mixins": "6.2.3",
    "postcss-simple-vars": "^5.0.2",
-    "prettier": "2.0.2",
+    "prettier": "2.0.4",
    "run-with-testrpc": "0.3.1",
-    "style-loader": "1.1.3",
+    "style-loader": "1.1.4",
    "terser-webpack-plugin": "2.3.5",
-    "truffle": "5.1.19",
+    "truffle": "5.1.21",
    "truffle-contract": "4.0.31",
    "truffle-solidity-loader": "0.1.32",
-    "url-loader": "4.0.0",
+    "url-loader": "4.1.0",
    "webpack": "4.42.1",
-    "webpack-bundle-analyzer": "3.6.1",
+    "webpack-bundle-analyzer": "3.7.0",
    "webpack-cli": "3.3.11",
    "webpack-dev-server": "3.10.3",
    "webpack-manifest-plugin": "2.2.0"
--- a/src/components/Sidebar/index.jsx
+++ b/src/components/Sidebar/index.jsx
@ -1,5 +1,4 @@
 // @flow
-import ClickAwayListener from '@material-ui/core/ClickAwayListener'
 import Drawer from '@material-ui/core/Drawer'
 import SearchIcon from '@material-ui/icons/Search'
 import { List } from 'immutable'
@ -90,51 +89,49 @@ const Sidebar = ({ children, currentSafe, defaultSafe, safes, setDefaultSafeActi

  return (
    <SidebarContext.Provider value={{ isOpen, toggleSidebar }}>
-      <ClickAwayListener onClickAway={toggleSidebar}>
-        <Drawer
-          classes={{ paper: classes.sidebarPaper }}
-          className={classes.sidebar}
-          ModalProps={{ onBackdropClick: toggleSidebar }}
-          onKeyDown={handleEsc}
-          open={isOpen}
-        >
-          <Row align="center" className={classes.topComponents}>
-            <Row align="center" className={classes.searchWrapper}>
-              <SearchIcon className={classes.searchIcon} />
-              <SearchBar
-                classes={searchClasses}
-                onCancelSearch={handleFilterCancel}
-                onChange={handleFilterChange}
-                placeholder="Search by name or address"
-                searchIcon={<div />}
-                value={filter}
-              />
-            </Row>
-            <Divider className={classes.divider} />
-            <Spacer className={classes.spacer} />
-            <Button
-              className={classes.addSafeBtn}
-              color="primary"
-              component={Link}
-              onClick={toggleSidebar}
-              size="small"
-              to={WELCOME_ADDRESS}
-              variant="contained"
-            >
-              + Add Safe
-            </Button>
-            <Spacer />
+      <Drawer
+        classes={{ paper: classes.sidebarPaper }}
+        className={classes.sidebar}
+        ModalProps={{ onBackdropClick: toggleSidebar }}
+        onKeyDown={handleEsc}
+        open={isOpen}
+      >
+        <Row align="center" className={classes.topComponents}>
+          <Row align="center" className={classes.searchWrapper}>
+            <SearchIcon className={classes.searchIcon} />
+            <SearchBar
+              classes={searchClasses}
+              onCancelSearch={handleFilterCancel}
+              onChange={handleFilterChange}
+              placeholder="Search by name or address"
+              searchIcon={<div />}
+              value={filter}
+            />
          </Row>
-          <Hairline />
-          <SafeList
-            currentSafe={currentSafe}
-            defaultSafe={defaultSafe}
-            onSafeClick={toggleSidebar}
-            safes={filteredSafes}
-            setDefaultSafe={setDefaultSafeAction}
-          />
-        </Drawer>
-      </ClickAwayListener>
+          <Divider className={classes.divider} />
+          <Spacer className={classes.spacer} />
+          <Button
+            className={classes.addSafeBtn}
+            color="primary"
+            component={Link}
+            onClick={toggleSidebar}
+            size="small"
+            to={WELCOME_ADDRESS}
+            variant="contained"
+          >
+            + Add Safe
+          </Button>
+          <Spacer />
+        </Row>
+        <Hairline />
+        <SafeList
+          currentSafe={currentSafe}
+          defaultSafe={defaultSafe}
+          onSafeClick={toggleSidebar}
+          safes={filteredSafes}
+          setDefaultSafe={setDefaultSafeAction}
+        />
+      </Drawer>
      {children}
    </SidebarContext.Provider>
  )
--- a/src/logic/contracts/safeContracts.js
+++ b/src/logic/contracts/safeContracts.js
@ -83,7 +83,6 @@ export const estimateGasForDeployingSafe = async (
  numConfirmations: number,
  userAccount: string,
 ) => {
-  console.log(proxyFactoryMaster)
  const gnosisSafeData = await safeMaster.contract.methods
    .setup(safeAccounts, numConfirmations, ZERO_ADDRESS, '0x', DEFAULT_FALLBACK_HANDLER_ADDRESS, ZERO_ADDRESS, 0, ZERO_ADDRESS)
    .encodeABI()
--- a/src/logic/safe/transactions/gas.js
+++ b/src/logic/safe/transactions/gas.js
@ -1,92 +1,5 @@
 // @flow
-import { BigNumber } from 'bignumber.js'
-
 import { getGnosisSafeInstanceAt } from '~/logic/contracts/safeContracts'
-import { EMPTY_DATA } from '~/logic/wallets/ethTransactions'
-import { getWeb3 } from '~/logic/wallets/getWeb3'
-
-const estimateDataGasCosts = (data) => {
-  const reducer = (accumulator, currentValue) => {
-    if (currentValue === EMPTY_DATA) {
-      return accumulator + 0
-    }
-
-    if (currentValue === '00') {
-      return accumulator + 4
-    }
-
-    return accumulator + 68
-  }
-
-  return data.match(/.{2}/g).reduce(reducer, 0)
-}
-
-// https://docs.gnosis.io/safe/docs/docs4/#safe-transaction-data-gas-estimation
-// https://github.com/gnosis/safe-contracts/blob/a97c6fd24f79c0b159ddd25a10a2ebd3ea2ef926/test/utils/execution.js
-export const estimateDataGas = (
-  safe: any,
-  to: string,
-  valueInWei: number,
-  from: string,
-  data: string,
-  operation: number,
-  txGasEstimate: number,
-  gasToken: number,
-  nonce: number,
-  signatureCount: number,
-  refundReceiver: number,
-) => {
-  // numbers < 256 are 192 -> 31 * 4 + 68
-  // numbers < 65k are 256 -> 30 * 4 + 2 * 68
-  // For signature array length and dataGasEstimate we already calculated
-  // the 0 bytes so we just add 64 for each non-zero byte
-  const gasPrice = 0 // no need to get refund when we submit txs to metamask
-  const signatureCost = signatureCount * (68 + 2176 + 2176 + 6000) // array count (3 -> r, s, v) * signature count
-
-  // https://docs.gnosis.io/safe/docs/docs5/#pre-validated-signatures
-  const sigs = `0x000000000000000000000000${from.replace(
-    '0x',
-    '',
-  )}000000000000000000000000000000000000000000000000000000000000000001`
-  const payload = safe.contract.methods
-    .execTransaction(to, valueInWei, data, operation, txGasEstimate, 0, gasPrice, gasToken, refundReceiver, sigs)
-    .encodeABI()
-
-  // eslint-disable-next-line
-  const dataGasEstimate = estimateDataGasCosts(payload) + signatureCost + (nonce > 0 ? 5000 : 20000) + 1500 // 1500 -> hash generation costs
-
-  return dataGasEstimate + 32000 // Add aditional gas costs (e.g. base tx costs, transfer costs)
-}
-
-export const generateTxGasEstimateFrom = async (
-  safe: any,
-  safeAddress: string,
-  data: string,
-  to: string,
-  valueInWei: number,
-  operation: number,
-) => {
-  try {
-    let safeInstance = safe
-    if (!safeInstance) {
-      safeInstance = await getGnosisSafeInstanceAt(safeAddress)
-    }
-
-    const estimateData = safeInstance.contract.methods.requiredTxGas(to, valueInWei, data, operation).encodeABI()
-    const estimateResponse = await getWeb3().eth.call({
-      to: safeAddress,
-      from: safeAddress,
-      data: estimateData,
-    })
-    const txGasEstimate = new BigNumber(estimateResponse.substring(138), 16)
-
-    // Add 10k else we will fail in case of nested calls
-    return txGasEstimate.toNumber() + 10000
-  } catch (error) {
-    console.error('Error calculating tx gas estimation', error)
-    return 0
-  }
-}

 export const calculateTxFee = async (
  safe: any,
--- a/src/logic/safe/transactions/gasNew.js
+++ b/src/logic/safe/transactions/gasNew.js
@ -1,14 +1,32 @@
 // @flow
 import GnosisSafeSol from '@gnosis.pm/safe-contracts/build/contracts/GnosisSafe.json'
+import { BigNumber } from 'bignumber.js'

 import { CALL } from '.'

+import { getGnosisSafeInstanceAt } from '~/logic/contracts/safeContracts'
 import { generateSignaturesFromTxConfirmations } from '~/logic/safe/safeTxSigner'
 import { ZERO_ADDRESS } from '~/logic/wallets/ethAddresses'
-import { calculateGasOf, calculateGasPrice } from '~/logic/wallets/ethTransactions'
+import { EMPTY_DATA, calculateGasOf, calculateGasPrice } from '~/logic/wallets/ethTransactions'
 import { getAccountFrom, getWeb3 } from '~/logic/wallets/getWeb3'
 import { type Transaction } from '~/routes/safe/store/models/transaction'

+const estimateDataGasCosts = (data) => {
+  const reducer = (accumulator, currentValue) => {
+    if (currentValue === EMPTY_DATA) {
+      return accumulator + 0
+    }
+
+    if (currentValue === '00') {
+      return accumulator + 4
+    }
+
+    return accumulator + 16
+  }
+
+  return data.match(/.{2}/g).reduce(reducer, 0)
+}
+
 export const estimateTxGasCosts = async (
  safeAddress: string,
  to: string,
@ -58,3 +76,70 @@ export const estimateTxGasCosts = async (
    return 10000
  }
 }
+
+export const estimateSafeTxGas = async (
+  safe: any,
+  safeAddress: string,
+  data: string,
+  to: string,
+  valueInWei: number | string,
+  operation: number,
+) => {
+  try {
+    let safeInstance = safe
+    if (!safeInstance) {
+      safeInstance = await getGnosisSafeInstanceAt(safeAddress)
+    }
+
+    const web3 = await getWeb3()
+    const estimateData = safeInstance.contract.methods.requiredTxGas(to, valueInWei, data, operation).encodeABI()
+    const estimateResponse = await web3.eth.call({
+      to: safeAddress,
+      from: safeAddress,
+      data: estimateData,
+    })
+    const txGasEstimation = new BigNumber(estimateResponse.substring(138), 16).toNumber() + 10000
+
+    // 21000 - additional gas costs (e.g. base tx costs, transfer costs)
+    const dataGasEstimation = estimateDataGasCosts(estimateData) + 21000
+
+    const additionalGasBatches = [10000, 20000, 40000, 80000, 160000, 320000, 640000, 1280000, 2560000, 5120000]
+
+    const batch = new web3.BatchRequest()
+    const estimationRequests = additionalGasBatches.map(
+      (additionalGas) =>
+        new Promise((resolve) => {
+          const request = web3.eth.call.request(
+            {
+              to: safe.address,
+              from: safe.address,
+              data: estimateData,
+              gasPrice: 0,
+              gasLimit: txGasEstimation + dataGasEstimation + additionalGas,
+            },
+            (error, res) => {
+              resolve({
+                success: error || res === '0x' ? false : true,
+                estimation: txGasEstimation + additionalGas,
+              })
+            },
+          )
+
+          batch.add(request)
+        }),
+    )
+    batch.execute()
+
+    const estimationResponses = await Promise.all(estimationRequests)
+    const firstSuccessfulRequest = estimationResponses.find((res) => res.success)
+
+    if (firstSuccessfulRequest) {
+      return firstSuccessfulRequest.estimation
+    }
+
+    return 0
+  } catch (error) {
+    console.error('Error calculating tx gas estimation', error)
+    return 0
+  }
+}
--- a/src/routes/safe/store/actions/createTransaction.js
+++ b/src/routes/safe/store/actions/createTransaction.js
@ -13,6 +13,7 @@ import {
  getExecutionTransaction,
  saveTxToHistory,
 } from '~/logic/safe/transactions'
+import { estimateSafeTxGas } from '~/logic/safe/transactions/gasNew'
 import { SAFE_VERSION_FOR_OFFCHAIN_SIGNATURES, tryOffchainSigning } from '~/logic/safe/transactions/offchainSigner'
 import { getCurrentSafeVersion } from '~/logic/safe/utils/safeVersion'
 import { ZERO_ADDRESS } from '~/logic/wallets/ethAddresses'
@ -66,6 +67,7 @@ const createTransaction = ({
  const nonce = await getNewTxNonce(txNonce, lastTx, safeInstance)
  const isExecution = await shouldExecuteTransaction(safeInstance, nonce, lastTx)
  const safeVersion = await getCurrentSafeVersion(safeInstance)
+  const safeTxGas = await estimateSafeTxGas(safeInstance, safeAddress, txData, to, valueInWei, operation)

  // https://docs.gnosis.io/safe/docs/docs5/#pre-validated-signatures
  const sigs = `0x000000000000000000000000${from.replace(
@ -86,7 +88,7 @@ const createTransaction = ({
    data: txData,
    operation,
    nonce,
-    safeTxGas: 0,
+    safeTxGas,
    baseGas: 0,
    gasPrice: 0,
    gasToken: ZERO_ADDRESS,
--- a/yarn.lock
+++ b/yarn.lock