From b960ecefe8cabe9041a21de73e075250858a32c1 Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 22 Aug 2018 13:38:35 +0200 Subject: [PATCH 1/7] WA-521 Adding feature flag for use metamask provider as signer --- src/config/development.js | 8 +++++++- src/config/index.js | 13 ++++++++++++- src/config/names.js | 1 + src/config/production.js | 8 +++++++- src/config/testing.js | 8 +++++++- 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/config/development.js b/src/config/development.js index 9473254a..b1630ac8 100644 --- a/src/config/development.js +++ b/src/config/development.js @@ -1,10 +1,16 @@ // @flow -import { TX_SERVICE_HOST, ENABLED_TX_SERVICE_MODULES, ENABLED_TX_SERVICE_REMOVAL_SENDER } from '~/config/names' +import { + TX_SERVICE_HOST, + ENABLED_TX_SERVICE_MODULES, + ENABLED_TX_SERVICE_REMOVAL_SENDER, + SIGNATURES_VIA_METAMASK, +} from '~/config/names' const devConfig = { [TX_SERVICE_HOST]: 'http://localhost:8000/api/v1/', [ENABLED_TX_SERVICE_MODULES]: false, [ENABLED_TX_SERVICE_REMOVAL_SENDER]: false, + [SIGNATURES_VIA_METAMASK]: true, } export default devConfig diff --git a/src/config/index.js b/src/config/index.js index 192ecdbf..649ab512 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -1,6 +1,11 @@ // @flow import { ensureOnce } from '~/utils/singleton' -import { TX_SERVICE_HOST, ENABLED_TX_SERVICE_MODULES, ENABLED_TX_SERVICE_REMOVAL_SENDER } from '~/config/names' +import { + TX_SERVICE_HOST, + ENABLED_TX_SERVICE_MODULES, + ENABLED_TX_SERVICE_REMOVAL_SENDER, + SIGNATURES_VIA_METAMASK, +} from '~/config/names' import devConfig from './development' import testConfig from './testing' import prodConfig from './production' @@ -38,3 +43,9 @@ export const allowedRemoveSenderInTxHistoryService = () => { return config[ENABLED_TX_SERVICE_REMOVAL_SENDER] } + +export const signaturesViaMetamask = () => { + const config = getConfig() + + return config[SIGNATURES_VIA_METAMASK] +} diff --git a/src/config/names.js b/src/config/names.js index 50fbdbf4..a3421248 100644 --- a/src/config/names.js +++ b/src/config/names.js @@ -3,3 +3,4 @@ export const TX_SERVICE_HOST = 'tsh' export const ENABLED_TX_SERVICE_MODULES = 'tsm' export const ENABLED_TX_SERVICE_REMOVAL_SENDER = 'trs' +export const SIGNATURES_VIA_METAMASK = 'svm' diff --git a/src/config/production.js b/src/config/production.js index 5544c438..92ef00d6 100644 --- a/src/config/production.js +++ b/src/config/production.js @@ -1,10 +1,16 @@ // @flow -import { TX_SERVICE_HOST, ENABLED_TX_SERVICE_MODULES, ENABLED_TX_SERVICE_REMOVAL_SENDER } from '~/config/names' +import { + TX_SERVICE_HOST, + ENABLED_TX_SERVICE_MODULES, + ENABLED_TX_SERVICE_REMOVAL_SENDER, + SIGNATURES_VIA_METAMASK, +} from '~/config/names' const prodConfig = { [TX_SERVICE_HOST]: 'https://safe-transaction-history.dev.gnosisdev.com/api/v1/', [ENABLED_TX_SERVICE_MODULES]: false, [ENABLED_TX_SERVICE_REMOVAL_SENDER]: false, + [SIGNATURES_VIA_METAMASK]: true, } export default prodConfig diff --git a/src/config/testing.js b/src/config/testing.js index cb3900a2..9aba3dba 100644 --- a/src/config/testing.js +++ b/src/config/testing.js @@ -1,10 +1,16 @@ // @flow -import { TX_SERVICE_HOST, ENABLED_TX_SERVICE_MODULES, ENABLED_TX_SERVICE_REMOVAL_SENDER } from '~/config/names' +import { + TX_SERVICE_HOST, + ENABLED_TX_SERVICE_MODULES, + ENABLED_TX_SERVICE_REMOVAL_SENDER, + SIGNATURES_VIA_METAMASK, +} from '~/config/names' const testConfig = { [TX_SERVICE_HOST]: 'http://localhost:8000/api/v1/', [ENABLED_TX_SERVICE_MODULES]: false, [ENABLED_TX_SERVICE_REMOVAL_SENDER]: false, + [SIGNATURES_VIA_METAMASK]: true, } export default testConfig From 6c7b4877240dd834be1d88c7aa340c4d3cf4662b Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 22 Aug 2018 13:39:08 +0200 Subject: [PATCH 2/7] WA-521 Mock signatures service in localstorage --- src/utils/localStorage/signatures.js | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 src/utils/localStorage/signatures.js diff --git a/src/utils/localStorage/signatures.js b/src/utils/localStorage/signatures.js new file mode 100644 index 00000000..ab1fea62 --- /dev/null +++ b/src/utils/localStorage/signatures.js @@ -0,0 +1,32 @@ +// @flow +import { Map } from 'immutable' +import { load } from '~/utils/localStorage' + +const getSignaturesKeyFrom = (safeAddress: string) => `TXS-SIGNATURES-${safeAddress}` + +export const storeSignature = (safeAddress: string, nonce: number, signature: string) => { + const signaturesKey = getSignaturesKeyFrom(safeAddress) + const subjects = Map(load(signaturesKey)) || Map() + + try { + const key = `${nonce}` + const existingSignatures = subjects.get(key) + const signatures = existingSignatures ? existingSignatures + signature : signature + const updatedSubjects = subjects.set(key, signatures) + const serializedState = JSON.stringify(updatedSubjects) + localStorage.setItem(signaturesKey, serializedState) + } catch (err) { + // eslint-disable-next-line + console.log('Error storing signatures in localstorage') + } +} + +export const getSignaturesFrom = (safeAddress: string, nonce: number) => { + const key = getSignaturesKeyFrom(safeAddress) + const data: any = load(key) + + const signatures = data ? Map(data) : Map() + const txSigs = signatures.get(String(nonce)) || '' + + return `0x${txSigs}` +} From 9d1098295ac965072c743a44bf1cda967b362f60 Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 22 Aug 2018 13:40:10 +0200 Subject: [PATCH 3/7] WA-521 Safe TX signature module --- src/logic/safe/safeTxSigner.js | 161 +++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 src/logic/safe/safeTxSigner.js diff --git a/src/logic/safe/safeTxSigner.js b/src/logic/safe/safeTxSigner.js new file mode 100644 index 00000000..d6b0b92d --- /dev/null +++ b/src/logic/safe/safeTxSigner.js @@ -0,0 +1,161 @@ +// @flow +import { getWeb3 } from '~/logic/wallets/getWeb3' +import { promisify } from '~/utils/promisify' +import { BigNumber } from 'bignumber.js' +import { EMPTY_DATA } from '~/logic/wallets/ethTransactions' +import { getSignaturesFrom } from '~/utils/localStorage/signatures' + +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) +} + + +export const estimateDataGas = ( + safe: any, + to: string, + valueInWei: number, + data: string, + operation: number, + txGasEstimate: number, + gasToken: number, + nonce: number, + signatureCount: 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) // array count (3 -> r, s, v) * signature count + + const sigs = getSignaturesFrom(safe.address, nonce) + const payload = safe.contract.execTransactionAndPaySubmitter + .getData(to, valueInWei, data, operation, txGasEstimate, 0, gasPrice, gasToken, sigs) + + let dataGasEstimate = estimateDataGasCosts(payload) + signatureCost + if (dataGasEstimate > 65536) { + dataGasEstimate += 64 + } else { + dataGasEstimate += 128 + } + return dataGasEstimate + 34000 // Add aditional gas costs (e.g. base tx costs, transfer costs) +} + +// eslint-disable-next-line +export const generateTxGasEstimateFrom = async ( + safe: any, + safeAddress: string, + data: string, + to: string, + valueInWei: number, + operation: number, +) => { + try { + const estimateData = safe.contract.requiredTxGas.getData(to, valueInWei, data, operation) + const estimateResponse = await promisify(cb => getWeb3().eth.call({ + to: safeAddress, + from: safeAddress, + data: estimateData, + }, cb)) + const txGasEstimate = new BigNumber(estimateResponse.substring(138), 16) + + // Add 10k else we will fail in case of nested calls + return Promise.resolve(txGasEstimate.toNumber() + 10000) + } catch (error) { + // eslint-disable-next-line + console.log("Error calculating tx gas estimation " + error) + return Promise.resolve(0) + } +} + +const generateTypedDataFrom = async ( + safe: any, + safeAddress: string, + to: string, + valueInWei: number, + nonce: number, + data: string, + operation: number, + txGasEstimate: number, +) => { + const txGasToken = 0 + // const threshold = await safe.getThreshold() + // estimateDataGas(safe, to, valueInWei, data, operation, txGasEstimate, txGasToken, nonce, threshold) + const dataGasEstimate = 0 + const gasPrice = 0 + const typedData = { + types: { + EIP712Domain: [ + { + type: 'address', + name: 'verifyingContract', + }, + ], + PersonalSafeTx: [ + { type: 'address', name: 'to' }, + { type: 'uint256', name: 'value' }, + { type: 'bytes', name: 'data' }, + { type: 'uint8', name: 'operation' }, + { type: 'uint256', name: 'safeTxGas' }, + { type: 'uint256', name: 'dataGas' }, + { type: 'uint256', name: 'gasPrice' }, + { type: 'address', name: 'gasToken' }, + { type: 'uint256', name: 'nonce' }, + ], + }, + domain: { + verifyingContract: safeAddress, + }, + primaryType: 'PersonalSafeTx', + message: { + to, + value: valueInWei, + data, + operation, + safeTxGas: txGasEstimate, + dataGas: dataGasEstimate, + gasPrice, + gasToken: txGasToken, + nonce, + }, + } + + return typedData +} + +export const generateMetamaskSignature = async ( + safe: any, + safeAddress: string, + sender: string, + to: string, + valueInWei: number, + nonce: number, + data: string, + operation: number, + txGasEstimate: number, +) => { + const web3 = getWeb3() + const typedData = + await generateTypedDataFrom(safe, safeAddress, to, valueInWei, nonce, data, operation, txGasEstimate) + const signedTypedData = { + jsonrpc: '2.0', + method: 'eth_signTypedData', + params: [sender, typedData], + id: Date.now(), + } + const txSignedResponse = await promisify(cb => web3.currentProvider.sendAsync(signedTypedData, cb)) + + return txSignedResponse.result.replace(EMPTY_DATA, '') +} From b1c281b6d2a04f89abbc9391d7443745c008f175 Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 22 Aug 2018 13:41:48 +0200 Subject: [PATCH 4/7] WA-521 Use Personal Safe contract in case we use signatures validation via provider --- src/logic/contracts/safeContracts.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/logic/contracts/safeContracts.js b/src/logic/contracts/safeContracts.js index a3b16d72..730cc3c0 100644 --- a/src/logic/contracts/safeContracts.js +++ b/src/logic/contracts/safeContracts.js @@ -4,10 +4,12 @@ import { ensureOnce } from '~/utils/singleton' import { getWeb3 } from '~/logic/wallets/getWeb3' import { promisify } from '~/utils/promisify' import GnosisSafeSol from '#/GnosisSafeTeamEdition.json' +import GnosisPersonalSafeSol from '#/GnosisSafePersonalEdition.json' import ProxyFactorySol from '#/ProxyFactory.json' import CreateAndAddModules from '#/CreateAndAddModules.json' import DailyLimitModule from '#/DailyLimitModule.json' import { calculateGasOf, calculateGasPrice, EMPTY_DATA } from '~/logic/wallets/ethTransactions' +import { signaturesViaMetamask } from '~/config' let proxyFactoryMaster let createAndAddModuleMaster @@ -30,8 +32,14 @@ function createAndAddModulesData(dataArray) { return dataArray.reduce((acc, data) => acc + mw.setup.getData(data).substr(74), EMPTY_DATA) } - const createGnosisSafeContract = (web3: any) => { + if (signaturesViaMetamask()) { + const gnosisSafe = contract(GnosisPersonalSafeSol) + gnosisSafe.setProvider(web3.currentProvider) + + return gnosisSafe + } + const gnosisSafe = contract(GnosisSafeSol) gnosisSafe.setProvider(web3.currentProvider) From c0970f3906c2e798a6a60763524fa39ce796debc Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 22 Aug 2018 13:43:43 +0200 Subject: [PATCH 5/7] WA-521 Logic for handling txs via provider signatures --- src/logic/safe/safeBlockchainOperations.js | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/logic/safe/safeBlockchainOperations.js b/src/logic/safe/safeBlockchainOperations.js index 650775c9..efa2d531 100644 --- a/src/logic/safe/safeBlockchainOperations.js +++ b/src/logic/safe/safeBlockchainOperations.js @@ -3,6 +3,9 @@ import { calculateGasOf, checkReceiptStatus, calculateGasPrice } from '~/logic/w import { type Operation, submitOperation } from '~/logic/safe/safeTxHistory' import { getDailyLimitModuleFrom } from '~/logic/contracts/dailyLimitContracts' import { getSafeEthereumInstance } from '~/logic/safe/safeFrontendOperations' +import { generateMetamaskSignature, generateTxGasEstimateFrom, estimateDataGas } from '~/logic/safe/safeTxSigner' +import { storeSignature, getSignaturesFrom } from '~/utils/localStorage/signatures' +import { signaturesViaMetamask } from '~/config' export const approveTransaction = async ( safeAddress: string, @@ -15,6 +18,14 @@ export const approveTransaction = async ( ) => { const gasPrice = await calculateGasPrice() + if (signaturesViaMetamask()) { + const safe = await getSafeEthereumInstance(safeAddress) + const txGasEstimate = await generateTxGasEstimateFrom(safe, safeAddress, data, to, valueInWei, operation) + const signature = + await generateMetamaskSignature(safe, safeAddress, sender, to, valueInWei, nonce, data, operation, txGasEstimate) + storeSignature(safeAddress, nonce, signature) + } + const gnosisSafe = await getSafeEthereumInstance(safeAddress) const txData = gnosisSafe.contract.approveTransactionWithParameters.getData(to, valueInWei, data, operation, nonce) const gas = await calculateGasOf(txData, sender, safeAddress) @@ -39,6 +50,40 @@ export const executeTransaction = async ( ) => { const gasPrice = await calculateGasPrice() + if (signaturesViaMetamask()) { + const safe = await getSafeEthereumInstance(safeAddress) + const txGasEstimate = await generateTxGasEstimateFrom(safe, safeAddress, data, to, valueInWei, operation) + const signature = + await generateMetamaskSignature(safe, safeAddress, sender, to, valueInWei, nonce, data, operation, txGasEstimate) + storeSignature(safeAddress, nonce, signature) + + const sigs = getSignaturesFrom(safeAddress, nonce) + + const threshold = await safe.getThreshold() + const gas = await estimateDataGas(safe, to, valueInWei, data, operation, txGasEstimate, 0, nonce, Number(threshold)) + const numOwners = await safe.getOwners() + const gasIncludingRemovingStoreUpfront = gas + txGasEstimate + (numOwners.length * 15000) + + const txReceipt = await safe.execTransactionAndPaySubmitter( + to, + valueInWei, + data, + operation, + txGasEstimate, + 0, + 0, + 0, + sigs, + { from: sender, gas: gasIncludingRemovingStoreUpfront, gasPrice }, + ) + + const txHash = txReceipt.tx + await checkReceiptStatus(txHash) + // await submitOperation(safeAddress, to, valueInWei, data, operation, nonce, txHash, sender, 'execution') + + return txHash + } + const gnosisSafe = await getSafeEthereumInstance(safeAddress) const txConfirmationData = gnosisSafe.contract.execTransactionIfApproved.getData(to, valueInWei, data, operation, nonce) From cd91609380c1317453c91c72c6c51ba67bbc3736 Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 22 Aug 2018 13:44:02 +0200 Subject: [PATCH 6/7] WA-521 Instructions for loging errors --- src/test/utils/ethereumErrors.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/utils/ethereumErrors.js b/src/test/utils/ethereumErrors.js index 380e2ef6..ba8d874e 100644 --- a/src/test/utils/ethereumErrors.js +++ b/src/test/utils/ethereumErrors.js @@ -4,6 +4,9 @@ import abi from 'ethereumjs-abi' import { promisify } from '~/utils/promisify' /* +console.log(`to[${to}] \n\n valieInWei[${valueInWei}] \n\n + data[${data}] \n\n operation[${operation}] \n\n sigs[${sigs}]`) + const gnosisSafe = await getSafeEthereumInstance(address) await printOutApprove("Remove owner 3", address, await gnosisSafe.getOwners(), tx.get('data'), tx.get('nonce')) const txData = From 306099ff899651da8dc686b241646ee01edc930a Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 22 Aug 2018 16:27:54 +0200 Subject: [PATCH 7/7] WA-521 Disabling metamask signatures feature --- src/config/development.js | 2 +- src/config/production.js | 2 +- src/config/testing.js | 2 +- src/test/safe.redux.transactions.test.js | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/config/development.js b/src/config/development.js index b1630ac8..969a0a99 100644 --- a/src/config/development.js +++ b/src/config/development.js @@ -10,7 +10,7 @@ const devConfig = { [TX_SERVICE_HOST]: 'http://localhost:8000/api/v1/', [ENABLED_TX_SERVICE_MODULES]: false, [ENABLED_TX_SERVICE_REMOVAL_SENDER]: false, - [SIGNATURES_VIA_METAMASK]: true, + [SIGNATURES_VIA_METAMASK]: false, } export default devConfig diff --git a/src/config/production.js b/src/config/production.js index 92ef00d6..7666dc14 100644 --- a/src/config/production.js +++ b/src/config/production.js @@ -10,7 +10,7 @@ const prodConfig = { [TX_SERVICE_HOST]: 'https://safe-transaction-history.dev.gnosisdev.com/api/v1/', [ENABLED_TX_SERVICE_MODULES]: false, [ENABLED_TX_SERVICE_REMOVAL_SENDER]: false, - [SIGNATURES_VIA_METAMASK]: true, + [SIGNATURES_VIA_METAMASK]: false, } export default prodConfig diff --git a/src/config/testing.js b/src/config/testing.js index 9aba3dba..ef5d23be 100644 --- a/src/config/testing.js +++ b/src/config/testing.js @@ -10,7 +10,7 @@ const testConfig = { [TX_SERVICE_HOST]: 'http://localhost:8000/api/v1/', [ENABLED_TX_SERVICE_MODULES]: false, [ENABLED_TX_SERVICE_REMOVAL_SENDER]: false, - [SIGNATURES_VIA_METAMASK]: true, + [SIGNATURES_VIA_METAMASK]: false, } export default testConfig diff --git a/src/test/safe.redux.transactions.test.js b/src/test/safe.redux.transactions.test.js index b33bb241..cde151fb 100644 --- a/src/test/safe.redux.transactions.test.js +++ b/src/test/safe.redux.transactions.test.js @@ -12,6 +12,7 @@ import { promisify } from '~/utils/promisify' import { getWeb3 } from '~/logic/wallets/getWeb3' import { safeTransactionsSelector } from '~/routes/safe/store/selectors' import fetchSafe from '~/routes/safe/store/actions/fetchSafe' +import { signaturesViaMetamask } from '~/config' import { testTransactionFrom, testSizeOfTransactions } from './utils/historyServiceHelper' @@ -34,7 +35,7 @@ describe('Transactions Suite', () => { const gnosisSafe = await getSafeEthereumInstance(safeAddress) const firstTxData = gnosisSafe.contract.addOwnerWithThreshold.getData(accounts[1], 2) const executor = accounts[0] - const nonce = Date.now() + const nonce = signaturesViaMetamask() ? await gnosisSafe.nonce() : Date.now() const firstTxHash = await createTransaction(safe, 'Add Owner Second account', safeAddress, 0, nonce, executor, firstTxData) await store.dispatch(fetchSafe(safe)) safe = getSafeFrom(store.getState(), safeAddress)