From 1e9199db3238cdae215838a26d082b46b85f9eb3 Mon Sep 17 00:00:00 2001 From: apanizo Date: Tue, 5 Jun 2018 13:27:14 +0200 Subject: [PATCH 01/10] WA-238 Adding logic (including tests) for handling change threshold --- config/jest/jest.setup.js | 2 +- .../AddTransaction/createTransactions.js | 16 ++- .../AddTransaction/test/transactions.test.js | 34 ++--- .../AddTransaction/test/transactionsHelper.js | 3 +- .../Transactions/processTransactions.js | 14 ++- src/routes/safe/store/model/transaction.js | 2 + src/routes/safe/test/Safe.threshold.test.js | 117 ++++++++++++++++++ 7 files changed, 157 insertions(+), 31 deletions(-) create mode 100644 src/routes/safe/test/Safe.threshold.test.js diff --git a/config/jest/jest.setup.js b/config/jest/jest.setup.js index 8d9161a7..c97ef811 100644 --- a/config/jest/jest.setup.js +++ b/config/jest/jest.setup.js @@ -1,2 +1,2 @@ // @flow -jest.setTimeout(30000) +jest.setTimeout(45000) diff --git a/src/routes/safe/component/AddTransaction/createTransactions.js b/src/routes/safe/component/AddTransaction/createTransactions.js index 7ac3956b..d0d83569 100644 --- a/src/routes/safe/component/AddTransaction/createTransactions.js +++ b/src/routes/safe/component/AddTransaction/createTransactions.js @@ -47,6 +47,7 @@ export const storeTransaction = ( tx: string, safeAddress: string, safeThreshold: number, + data: string, ) => { const notMinedWhenOneOwnerSafe = confirmations.count() === 1 && !tx if (notMinedWhenOneOwnerSafe) { @@ -54,7 +55,7 @@ export const storeTransaction = ( } const transaction: Transaction = makeTransaction({ - name, nonce, value, confirmations, destination, threshold: safeThreshold, tx, + name, nonce, value, confirmations, destination, threshold: safeThreshold, tx, data, }) const safeTransactions = load(TX_KEY) || {} @@ -82,10 +83,11 @@ const hasOneOwner = (safe: Safe) => { export const createTransaction = async ( safe: Safe, txName: string, - txDestination: string, + txDest: string, txValue: number, nonce: number, user: string, + data: string = '0x', ) => { const web3 = getWeb3() const GnosisSafe = await getGnosisSafeContract(web3) @@ -97,19 +99,21 @@ export const createTransaction = async ( const thresholdIsOne = safe.get('confirmations') === 1 if (hasOneOwner(safe) || thresholdIsOne) { - const txConfirmationData = gnosisSafe.contract.execTransactionIfApproved.getData(txDestination, valueInWei, '0x', CALL, nonce) + const txConfirmationData = + gnosisSafe.contract.execTransactionIfApproved.getData(txDest, valueInWei, data, CALL, nonce) const txHash = await executeTransaction(txConfirmationData, user, safeAddress) checkReceiptStatus(txHash) const executedConfirmations: List = buildExecutedConfirmationFrom(safe.get('owners'), user) - return storeTransaction(txName, nonce, txDestination, txValue, user, executedConfirmations, txHash, safeAddress, safe.get('confirmations')) + return storeTransaction(txName, nonce, txDest, txValue, user, executedConfirmations, txHash, safeAddress, safe.get('confirmations'), data) } - const txConfirmationData = gnosisSafe.contract.approveTransactionWithParameters.getData(txDestination, valueInWei, '0x', CALL, nonce) + const txConfirmationData = + gnosisSafe.contract.approveTransactionWithParameters.getData(txDest, valueInWei, data, CALL, nonce) const txConfirmationHash = await executeTransaction(txConfirmationData, user, safeAddress) checkReceiptStatus(txConfirmationHash) const confirmations: List = buildConfirmationsFrom(safe.get('owners'), user, txConfirmationHash) - return storeTransaction(txName, nonce, txDestination, txValue, user, confirmations, '', safeAddress, safe.get('confirmations')) + return storeTransaction(txName, nonce, txDest, txValue, user, confirmations, '', safeAddress, safe.get('confirmations'), data) } diff --git a/src/routes/safe/component/AddTransaction/test/transactions.test.js b/src/routes/safe/component/AddTransaction/test/transactions.test.js index 367403bf..19c1fdeb 100644 --- a/src/routes/safe/component/AddTransaction/test/transactions.test.js +++ b/src/routes/safe/component/AddTransaction/test/transactions.test.js @@ -33,7 +33,7 @@ describe('Transactions Suite', () => { const txName = 'Buy butteries for project' const nonce: number = 10 const confirmations: List = buildConfirmationsFrom(owners, 'foo', 'confirmationHash') - storeTransaction(txName, nonce, destination, value, 'foo', confirmations, '', safe.get('address'), safe.get('confirmations')) + storeTransaction(txName, nonce, destination, value, 'foo', confirmations, '', safe.get('address'), safe.get('confirmations'), '0x') // WHEN const transactions: Map> = loadSafeTransactions() @@ -45,7 +45,7 @@ describe('Transactions Suite', () => { if (!safeTransactions) { throw new Error() } testSizeOfTransactions(safeTransactions, 1) - testTransactionFrom(safeTransactions, 0, txName, nonce, value, 2, destination, 'foo', 'confirmationHash', owners.get(0), owners.get(1)) + testTransactionFrom(safeTransactions, 0, txName, nonce, value, 2, destination, '0x', 'foo', 'confirmationHash', owners.get(0), owners.get(1)) }) it('adds second confirmation to stored safe with one confirmation', async () => { @@ -55,12 +55,12 @@ describe('Transactions Suite', () => { const safeAddress = safe.get('address') const creator = 'foo' const confirmations: List = buildConfirmationsFrom(owners, creator, 'confirmationHash') - storeTransaction(firstTxName, firstNonce, destination, value, creator, confirmations, '', safeAddress, safe.get('confirmations')) + storeTransaction(firstTxName, firstNonce, destination, value, creator, confirmations, '', safeAddress, safe.get('confirmations'), '0x') const secondTxName = 'Buy printers for project' const secondNonce: number = firstNonce + 100 const secondConfirmations: List = buildConfirmationsFrom(owners, creator, 'confirmationHash') - storeTransaction(secondTxName, secondNonce, destination, value, creator, secondConfirmations, '', safeAddress, safe.get('confirmations')) + storeTransaction(secondTxName, secondNonce, destination, value, creator, secondConfirmations, '', safeAddress, safe.get('confirmations'), '0x') // WHEN const transactions: Map> = loadSafeTransactions() @@ -72,8 +72,8 @@ describe('Transactions Suite', () => { if (!safeTxs) { throw new Error() } testSizeOfTransactions(safeTxs, 2) - testTransactionFrom(safeTxs, 0, firstTxName, firstNonce, value, 2, destination, 'foo', 'confirmationHash', owners.get(0), owners.get(1)) - testTransactionFrom(safeTxs, 1, secondTxName, secondNonce, value, 2, destination, 'foo', 'confirmationHash', owners.get(0), owners.get(1)) + testTransactionFrom(safeTxs, 0, firstTxName, firstNonce, value, 2, destination, '0x', 'foo', 'confirmationHash', owners.get(0), owners.get(1)) + testTransactionFrom(safeTxs, 1, secondTxName, secondNonce, value, 2, destination, '0x', 'foo', 'confirmationHash', owners.get(0), owners.get(1)) }) it('adds second confirmation to stored safe having two safes with one confirmation each', async () => { @@ -82,7 +82,7 @@ describe('Transactions Suite', () => { const safeAddress = safe.address const creator = 'foo' const confirmations: List = buildConfirmationsFrom(owners, creator, 'confirmationHash') - storeTransaction(txName, nonce, destination, value, creator, confirmations, '', safeAddress, safe.get('confirmations')) + storeTransaction(txName, nonce, destination, value, creator, confirmations, '', safeAddress, safe.get('confirmations'), '0x') const secondSafe = SafeFactory.dailyLimitSafe(10, 2) const txSecondName = 'Buy batteris for Beta project' @@ -92,7 +92,7 @@ describe('Transactions Suite', () => { const secondConfirmations: List = buildConfirmationsFrom(secondSafe.get('owners'), secondCreator, 'confirmationHash') storeTransaction( txSecondName, txSecondNonce, destination, value, secondCreator, - secondConfirmations, '', secondSafeAddress, secondSafe.get('confirmations'), + secondConfirmations, '', secondSafeAddress, secondSafe.get('confirmations'), '0x', ) let transactions: Map> = loadSafeTransactions() @@ -112,7 +112,7 @@ describe('Transactions Suite', () => { const txConfirmations: List = buildConfirmationsFrom(owners, creator, 'secondConfirmationHash') storeTransaction( txFirstName, txFirstNonce, destination, value, creator, - txConfirmations, '', safe.get('address'), safe.get('confirmations'), + txConfirmations, '', safe.get('address'), safe.get('confirmations'), '0x', ) transactions = loadSafeTransactions() @@ -125,19 +125,19 @@ describe('Transactions Suite', () => { // Test 2 transactions of first safe testTransactionFrom( transactions.get(safe.address), 0, - txName, nonce, value, 2, destination, + txName, nonce, value, 2, destination, '0x', 'foo', 'confirmationHash', owners.get(0), owners.get(1), ) testTransactionFrom( transactions.get(safe.address), 1, - txFirstName, txFirstNonce, value, 2, destination, + txFirstName, txFirstNonce, value, 2, destination, '0x', 'foo', 'secondConfirmationHash', owners.get(0), owners.get(1), ) // Test one transaction of second safe testTransactionFrom( transactions.get(secondSafe.address), 0, - txSecondName, txSecondNonce, value, 2, destination, + txSecondName, txSecondNonce, value, 2, destination, '0x', '0x03db1a8b26d08df23337e9276a36b474510f0023', 'confirmationHash', secondSafe.get('owners').get(0), secondSafe.get('owners').get(1), ) }) @@ -148,10 +148,10 @@ describe('Transactions Suite', () => { const nonce: number = 10 const creator = 'foo' const confirmations: List = buildConfirmationsFrom(owners, creator, 'confirmationHash') - storeTransaction(txName, nonce, destination, value, creator, confirmations, '', safe.get('address'), safe.get('confirmations')) + storeTransaction(txName, nonce, destination, value, creator, confirmations, '', safe.get('address'), safe.get('confirmations'), '0x') // WHEN - const createTxFnc = () => storeTransaction(txName, nonce, destination, value, creator, confirmations, '', safe.get('address'), safe.get('confirmations')) + const createTxFnc = () => storeTransaction(txName, nonce, destination, value, creator, confirmations, '', safe.get('address'), safe.get('confirmations'), '0x') expect(createTxFnc).toThrow(/Transaction with same nonce/) }) @@ -161,7 +161,7 @@ describe('Transactions Suite', () => { const nonce: number = 10 const creator = 'foo' const confirmations: List = buildConfirmationsFrom(owners, creator, 'confirmationHash') - storeTransaction(txName, nonce, destination, value, creator, confirmations, '', safe.get('address'), safe.get('confirmations')) + storeTransaction(txName, nonce, destination, value, creator, confirmations, '', safe.get('address'), safe.get('confirmations'), '0x') // WHEN const transactions: Map> = loadSafeTransactions() @@ -185,7 +185,7 @@ describe('Transactions Suite', () => { const nonce: number = 10 const tx = '' const confirmations: List = buildExecutedConfirmationFrom(oneOwnerSafe.get('owners'), ownerName) - const createTxFnc = () => storeTransaction(txName, nonce, destination, value, ownerName, confirmations, tx, oneOwnerSafe.get('address'), oneOwnerSafe.get('confirmations')) + const createTxFnc = () => storeTransaction(txName, nonce, destination, value, ownerName, confirmations, tx, oneOwnerSafe.get('address'), oneOwnerSafe.get('confirmations'), '0x') expect(createTxFnc).toThrow(/The tx should be mined before storing it in safes with one owner/) }) @@ -197,7 +197,7 @@ describe('Transactions Suite', () => { const nonce: number = 10 const tx = 'validTxHash' const confirmations: List = buildExecutedConfirmationFrom(oneOwnerSafe.get('owners'), ownerName) - storeTransaction(txName, nonce, destination, value, ownerName, confirmations, tx, oneOwnerSafe.get('address'), oneOwnerSafe.get('confirmations')) + storeTransaction(txName, nonce, destination, value, ownerName, confirmations, tx, oneOwnerSafe.get('address'), oneOwnerSafe.get('confirmations'), '0x') // WHEN const safeTransactions: Map> = loadSafeTransactions() diff --git a/src/routes/safe/component/AddTransaction/test/transactionsHelper.js b/src/routes/safe/component/AddTransaction/test/transactionsHelper.js index 9b92df35..8ad22983 100644 --- a/src/routes/safe/component/AddTransaction/test/transactionsHelper.js +++ b/src/routes/safe/component/AddTransaction/test/transactionsHelper.js @@ -20,7 +20,7 @@ export const testSizeOfTransactions = (safeTxs: List | typeof undef export const testTransactionFrom = ( safeTxs: List | typeof undefined, pos: number, name: string, nonce: number, value: number, threshold: number, destination: string, - creator: string, txHash: string, + data: string, creator: string, txHash: string, firstOwner: Owner | typeof undefined, secondOwner: Owner | typeof undefined, ) => { if (!safeTxs) { throw new Error() } @@ -33,6 +33,7 @@ export const testTransactionFrom = ( expect(tx.get('destination')).toBe(destination) expect(tx.get('confirmations').count()).toBe(2) expect(tx.get('nonce')).toBe(nonce) + expect(tx.get('data')).toBe(data) const confirmations: List = tx.get('confirmations') const firstConfirmation: Confirmation | typeof undefined = confirmations.get(0) diff --git a/src/routes/safe/component/Transactions/processTransactions.js b/src/routes/safe/component/Transactions/processTransactions.js index dfbc64d2..32f0d123 100644 --- a/src/routes/safe/component/Transactions/processTransactions.js +++ b/src/routes/safe/component/Transactions/processTransactions.js @@ -20,9 +20,10 @@ export const updateTransaction = ( tx: string, safeAddress: string, safeThreshold: number, + data: string, ) => { const transaction: Transaction = makeTransaction({ - name, nonce, value, confirmations, destination, threshold: safeThreshold, tx, + name, nonce, value, confirmations, destination, threshold: safeThreshold, tx, data, }) const safeTransactions = load(TX_KEY) || {} @@ -36,7 +37,6 @@ export const updateTransaction = ( localStorage.setItem(TX_KEY, JSON.stringify(safeTransactions)) } -const getData = () => '0x' const getOperation = () => 0 const execTransaction = async ( @@ -45,8 +45,8 @@ const execTransaction = async ( txValue: number, nonce: number, executor: string, + data: string, ) => { - const data = getData() const CALL = getOperation() const web3 = getWeb3() const valueInWei = web3.toWei(txValue, 'ether') @@ -61,8 +61,8 @@ const execConfirmation = async ( txValue: number, nonce: number, executor: string, + data: string, ) => { - const data = getData() const CALL = getOperation() const web3 = getWeb3() const valueInWei = web3.toWei(txValue, 'ether') @@ -110,10 +110,11 @@ export const processTransaction = async ( const txName = tx.get('name') const txValue = tx.get('value') const txDestination = tx.get('destination') + const data = tx.get('data') const txHash = thresholdReached - ? await execTransaction(gnosisSafe, txDestination, txValue, nonce, userAddress) - : await execConfirmation(gnosisSafe, txDestination, txValue, nonce, userAddress) + ? await execTransaction(gnosisSafe, txDestination, txValue, nonce, userAddress, data) + : await execConfirmation(gnosisSafe, txDestination, txValue, nonce, userAddress, data) checkReceiptStatus(txHash) @@ -130,5 +131,6 @@ export const processTransaction = async ( thresholdReached ? txHash : '', safeAddress, threshold, + data, ) } diff --git a/src/routes/safe/store/model/transaction.js b/src/routes/safe/store/model/transaction.js index 83a664d0..e810ed00 100644 --- a/src/routes/safe/store/model/transaction.js +++ b/src/routes/safe/store/model/transaction.js @@ -11,6 +11,7 @@ export type TransactionProps = { confirmations: List, destination: string, tx: string, + data: string, } export const makeTransaction: RecordFactory = Record({ @@ -21,6 +22,7 @@ export const makeTransaction: RecordFactory = Record({ destination: '', tx: '', threshold: 0, + data: '', }) export type Transaction = RecordOf diff --git a/src/routes/safe/test/Safe.threshold.test.js b/src/routes/safe/test/Safe.threshold.test.js new file mode 100644 index 00000000..1ca38066 --- /dev/null +++ b/src/routes/safe/test/Safe.threshold.test.js @@ -0,0 +1,117 @@ +// @flow +import { aNewStore } from '~/store' +import { aDeployedSafe } from '~/routes/safe/store/test/builder/deployedSafe.builder' +import { getWeb3 } from '~/wallets/getWeb3' +import { sleep } from '~/utils/timer' +import { promisify } from '~/utils/promisify' +import { processTransaction } from '~/routes/safe/component/Transactions/processTransactions' +import { confirmationsTransactionSelector, safeSelector, safeTransactionsSelector } from '~/routes/safe/store/selectors/index' +import { getTransactionFromReduxStore } from '~/routes/safe/test/testMultisig' +import { buildMathPropsFrom } from '~/test/buildReactRouterProps' +import { createTransaction } from '~/routes/safe/component/AddTransaction/createTransactions' +import { getGnosisSafeContract } from '~/wallets/safeContracts' +import fetchTransactions from '~/routes/safe/store/actions/fetchTransactions' + +describe('React DOM TESTS > Change threshold', () => { + it('should update the threshold directly if safe has 1 threshold', async () => { + // GIVEN + const numOwners = 2 + const threshold = 1 + const store = aNewStore() + const address = await aDeployedSafe(store, 10, threshold, numOwners) + const accounts = await promisify(cb => getWeb3().eth.getAccounts(cb)) + const match: Match = buildMathPropsFrom(address) + const safe = safeSelector(store.getState(), { match }) + const web3 = getWeb3() + const GnosisSafe = await getGnosisSafeContract(web3) + const gnosisSafe = GnosisSafe.at(address) + + // WHEN + const nonce = Date.now() + const data = gnosisSafe.contract.changeThreshold.getData(2) + await createTransaction(safe, "Change Safe's threshold", address, 0, nonce, accounts[0], data) + await sleep(1500) + await store.dispatch(fetchTransactions()) + + // THEN + const transactions = safeTransactionsSelector(store.getState(), { safeAddress: address }) + expect(transactions.count()).toBe(1) + + const thresholdTx: Transaction = transactions.get(0) + expect(thresholdTx.get('tx')).not.toBe(null) + expect(thresholdTx.get('tx')).not.toBe(undefined) + expect(thresholdTx.get('tx')).not.toBe('') + + const safeThreshold = await gnosisSafe.getThreshold() + expect(Number(safeThreshold)).toEqual(2) + }) + + const changeThreshold = async (store, safeAddress, executor) => { + const tx = getTransactionFromReduxStore(store, safeAddress) + const confirmed = confirmationsTransactionSelector(store.getState(), { transaction: tx }) + const data = tx.get('data') + expect(data).not.toBe(null) + expect(data).not.toBe(undefined) + expect(data).not.toBe('') + await processTransaction(safeAddress, tx, confirmed, executor) + await sleep(1800) + } + + it('should wait for confirmation to update threshold when safe has 1+ threshold', async () => { + // GIVEN + const numOwners = 3 + const threshold = 2 + const store = aNewStore() + const address = await aDeployedSafe(store, 10, threshold, numOwners) + const accounts = await promisify(cb => getWeb3().eth.getAccounts(cb)) + const match: Match = buildMathPropsFrom(address) + const safe = safeSelector(store.getState(), { match }) + const web3 = getWeb3() + const GnosisSafe = await getGnosisSafeContract(web3) + const gnosisSafe = GnosisSafe.at(address) + + // WHEN + const nonce = Date.now() + const data = gnosisSafe.contract.changeThreshold.getData(3) + await createTransaction(safe, "Change Safe's threshold", address, 0, nonce, accounts[0], data) + await sleep(1500) + await store.dispatch(fetchTransactions()) + + let transactions = safeTransactionsSelector(store.getState(), { safeAddress: address }) + expect(transactions.count()).toBe(1) + + let thresholdTx: Transaction = transactions.get(0) + expect(thresholdTx.get('tx')).toBe('') + let firstOwnerConfirmation = thresholdTx.get('confirmations').get(0) + if (!firstOwnerConfirmation) throw new Error() + expect(firstOwnerConfirmation.get('status')).toBe(true) + let secondOwnerConfirmation = thresholdTx.get('confirmations').get(1) + if (!secondOwnerConfirmation) throw new Error() + expect(secondOwnerConfirmation.get('status')).toBe(false) + + let safeThreshold = await gnosisSafe.getThreshold() + expect(Number(safeThreshold)).toEqual(2) + + // THEN + await changeThreshold(store, address, accounts[1]) + safeThreshold = await gnosisSafe.getThreshold() + expect(Number(safeThreshold)).toEqual(3) + + await store.dispatch(fetchTransactions()) + sleep(1200) + transactions = safeTransactionsSelector(store.getState(), { safeAddress: address }) + expect(transactions.count()).toBe(1) + + thresholdTx = transactions.get(0) + expect(thresholdTx.get('tx')).not.toBe(undefined) + expect(thresholdTx.get('tx')).not.toBe(null) + expect(thresholdTx.get('tx')).not.toBe('') + + firstOwnerConfirmation = thresholdTx.get('confirmations').get(0) + if (!firstOwnerConfirmation) throw new Error() + expect(firstOwnerConfirmation.get('status')).toBe(true) + secondOwnerConfirmation = thresholdTx.get('confirmations').get(1) + if (!secondOwnerConfirmation) throw new Error() + expect(secondOwnerConfirmation.get('status')).toBe(true) + }) +}) From 45a5d1c4091e6db95d3b0058db5aab66b1c1653d Mon Sep 17 00:00:00 2001 From: apanizo Date: Tue, 5 Jun 2018 13:36:26 +0200 Subject: [PATCH 02/10] WA-235 Adding button Edit on confirmations section in Safe's view --- src/routes/safe/component/Safe/Confirmations.jsx | 13 ++++++++++++- src/routes/safe/component/Safe/index.jsx | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/routes/safe/component/Safe/Confirmations.jsx b/src/routes/safe/component/Safe/Confirmations.jsx index 91c192d9..faf23d45 100644 --- a/src/routes/safe/component/Safe/Confirmations.jsx +++ b/src/routes/safe/component/Safe/Confirmations.jsx @@ -4,12 +4,16 @@ import { ListItem } from 'material-ui/List' import Avatar from 'material-ui/Avatar' import DoneAll from 'material-ui-icons/DoneAll' import ListItemText from '~/components/List/ListItemText' +import Button from '~/components/layout/Button' type Props = { confirmations: number, + onEditThreshold: () => void, } -const Confirmations = ({ confirmations }: Props) => ( +const EDIT_THRESHOLD_BUTTON_TEXT = 'EDIT' + +const Confirmations = ({ confirmations, onEditThreshold }: Props) => ( @@ -19,6 +23,13 @@ const Confirmations = ({ confirmations }: Props) => ( secondary={`${confirmations} required confirmations per transaction`} cut /> + ) diff --git a/src/routes/safe/component/Safe/index.jsx b/src/routes/safe/component/Safe/index.jsx index 8e88fff2..4cb26bd7 100644 --- a/src/routes/safe/component/Safe/index.jsx +++ b/src/routes/safe/component/Safe/index.jsx @@ -69,7 +69,7 @@ class GnoSafe extends React.PureComponent { - + {}} />
From d3af885074fc4bb0dd0db732db4ef399dfb9e736 Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 6 Jun 2018 09:10:45 +0200 Subject: [PATCH 03/10] WA-235 Adapting buttton after threshold change --- src/components/forms/GnoForm/index.jsx | 2 +- .../AddTransaction/createTransactions.js | 10 +- src/routes/safe/component/Safe/index.jsx | 9 +- src/routes/safe/component/Threshold/index.jsx | 98 +++++++++++++++++++ .../safe/component/Threshold/selector.js | 11 +++ 5 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 src/routes/safe/component/Threshold/index.jsx create mode 100644 src/routes/safe/component/Threshold/selector.js diff --git a/src/components/forms/GnoForm/index.jsx b/src/components/forms/GnoForm/index.jsx index d51637f1..7dc29a35 100644 --- a/src/components/forms/GnoForm/index.jsx +++ b/src/components/forms/GnoForm/index.jsx @@ -29,7 +29,7 @@ const GnoForm = ({ render={({ handleSubmit, ...rest }) => (
{render(rest)} - {children(rest.submitting)} + {children(rest.submitting, rest.submitSucceeded)}
)} /> diff --git a/src/routes/safe/component/AddTransaction/createTransactions.js b/src/routes/safe/component/AddTransaction/createTransactions.js index d0d83569..2bc545cc 100644 --- a/src/routes/safe/component/AddTransaction/createTransactions.js +++ b/src/routes/safe/component/AddTransaction/createTransactions.js @@ -80,6 +80,12 @@ const hasOneOwner = (safe: Safe) => { return owners.count() === 1 } +export const getSafeEthereumInstance = async (safeAddress) => { + const web3 = getWeb3() + const GnosisSafe = await getGnosisSafeContract(web3) + return GnosisSafe.at(safeAddress) +} + export const createTransaction = async ( safe: Safe, txName: string, @@ -90,10 +96,8 @@ export const createTransaction = async ( data: string = '0x', ) => { const web3 = getWeb3() - const GnosisSafe = await getGnosisSafeContract(web3) const safeAddress = safe.get('address') - const gnosisSafe = GnosisSafe.at(safeAddress) - + const gnosisSafe = await getSafeEthereumInstance(safeAddress) const valueInWei = web3.toWei(txValue, 'ether') const CALL = 0 diff --git a/src/routes/safe/component/Safe/index.jsx b/src/routes/safe/component/Safe/index.jsx index 4cb26bd7..cbb9f894 100644 --- a/src/routes/safe/component/Safe/index.jsx +++ b/src/routes/safe/component/Safe/index.jsx @@ -12,6 +12,7 @@ import List from 'material-ui/List' import Withdrawn from '~/routes/safe/component/Withdrawn' import Transactions from '~/routes/safe/component/Transactions' import AddTransaction from '~/routes/safe/component/AddTransaction' +import Threshold from '~/routes/safe/component/Threshold' import Address from './Address' import Balance from './Balance' @@ -59,6 +60,12 @@ class GnoSafe extends React.PureComponent { this.setState({ component: }) } + onEditThreshold = () => { + const { safe } = this.props + + this.setState({ component: }) + } + render() { const { safe, balance } = this.props const { component } = this.state @@ -69,7 +76,7 @@ class GnoSafe extends React.PureComponent { - {}} /> +
diff --git a/src/routes/safe/component/Threshold/index.jsx b/src/routes/safe/component/Threshold/index.jsx new file mode 100644 index 00000000..86cca73c --- /dev/null +++ b/src/routes/safe/component/Threshold/index.jsx @@ -0,0 +1,98 @@ +// @flow +import * as React from 'react' +import Block from '~/components/layout/Block' +import Heading from '~/components/layout/Heading' +import Field from '~/components/forms/Field' +import TextField from '~/components/forms/TextField' +import GnoForm from '~/components/forms/GnoForm' +import { connect } from 'react-redux' +import Button from '~/components/layout/Button' +import Col from '~/components/layout/Col' +import Row from '~/components/layout/Row' +import { composeValidators, minValue, maxValue, mustBeInteger, required } from '~/components/forms/validator' +import { getSafeEthereumInstance, createTransaction } from '~/routes/safe/component/AddTransaction/createTransactions' +import { sleep } from '~/utils/timer' +import selector, { type SelectorProps } from './selector' + +type Props = SelectorProps & { + numOwners: number, + safe: Safe, +} + +const THRESHOLD_PARAM = 'threshold' + +const ThresholdComponent = ({ numOwners }: Props) => () => ( + + + {'Change safe\'s threshold'} + + + {`Actual number of owners: ${numOwners}`} + + + + + +) + +type State = { + initialValues: Object, +} + +class Threshold extends React.PureComponent { + state = { + initialValues: {}, + } + + onThreshold = async (values: Object) => { + const { safe, userAddress } = this.props // , fetchThreshold } = this.props + const newThreshold = values[THRESHOLD_PARAM] + const gnosisSafe = await getSafeEthereumInstance(safe.get('address')) + const nonce = Date.now() + const data = gnosisSafe.contract.changeThreshold.getData(newThreshold) + await createTransaction(safe, "Change Safe's threshold", safe.get('address'), 0, nonce, userAddress, data) + await sleep(1500) + // this.props.fetchThreshold(safe.get('address')) + } + + render() { + const { numOwners } = this.props + + return ( + + {(submitting: boolean, submitSucceeded: boolean) => ( + + + + + + )} + + ) + } +} + +export default connect(selector)(Threshold) diff --git a/src/routes/safe/component/Threshold/selector.js b/src/routes/safe/component/Threshold/selector.js new file mode 100644 index 00000000..9e7bfef1 --- /dev/null +++ b/src/routes/safe/component/Threshold/selector.js @@ -0,0 +1,11 @@ +// @flow +import { createStructuredSelector } from 'reselect' +import { userAccountSelector } from '~/wallets/store/selectors/index' + +export type SelectorProps = { + userAddress: userAccountSelector, +} + +export default createStructuredSelector({ + userAddress: userAccountSelector, +}) From ee0165ad28e0912358f59e029213e307744a655f Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 6 Jun 2018 09:15:42 +0200 Subject: [PATCH 04/10] WA-238 Adding callback after changing threshold --- src/routes/safe/component/Safe/index.jsx | 2 +- src/routes/safe/component/Threshold/index.jsx | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/routes/safe/component/Safe/index.jsx b/src/routes/safe/component/Safe/index.jsx index cbb9f894..788b2561 100644 --- a/src/routes/safe/component/Safe/index.jsx +++ b/src/routes/safe/component/Safe/index.jsx @@ -63,7 +63,7 @@ class GnoSafe extends React.PureComponent { onEditThreshold = () => { const { safe } = this.props - this.setState({ component: }) + this.setState({ component: }) } render() { diff --git a/src/routes/safe/component/Threshold/index.jsx b/src/routes/safe/component/Threshold/index.jsx index 86cca73c..0ed1da98 100644 --- a/src/routes/safe/component/Threshold/index.jsx +++ b/src/routes/safe/component/Threshold/index.jsx @@ -17,6 +17,7 @@ import selector, { type SelectorProps } from './selector' type Props = SelectorProps & { numOwners: number, safe: Safe, + onReset: () => void, } const THRESHOLD_PARAM = 'threshold' @@ -62,13 +63,13 @@ class Threshold extends React.PureComponent { const gnosisSafe = await getSafeEthereumInstance(safe.get('address')) const nonce = Date.now() const data = gnosisSafe.contract.changeThreshold.getData(newThreshold) - await createTransaction(safe, "Change Safe's threshold", safe.get('address'), 0, nonce, userAddress, data) + await createTransaction(safe, `Change Safe's threshold [${nonce}]`, safe.get('address'), 0, nonce, userAddress, data) await sleep(1500) // this.props.fetchThreshold(safe.get('address')) } render() { - const { numOwners } = this.props + const { numOwners, onReset } = this.props return ( { From fad5132a60d18368b7680b7cbd6619bce95f70ab Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 6 Jun 2018 09:28:32 +0200 Subject: [PATCH 05/10] WA-238 Adding actual threshold info in the component --- src/routes/safe/component/Threshold/index.jsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/routes/safe/component/Threshold/index.jsx b/src/routes/safe/component/Threshold/index.jsx index 0ed1da98..61c4c565 100644 --- a/src/routes/safe/component/Threshold/index.jsx +++ b/src/routes/safe/component/Threshold/index.jsx @@ -22,13 +22,13 @@ type Props = SelectorProps & { const THRESHOLD_PARAM = 'threshold' -const ThresholdComponent = ({ numOwners }: Props) => () => ( +const ThresholdComponent = ({ numOwners, safe }: Props) => () => ( {'Change safe\'s threshold'} - {`Actual number of owners: ${numOwners}`} + {`Safe's owners: ${numOwners} and Safe's threshold: ${safe.get('confirmations')}`} { } render() { - const { numOwners, onReset } = this.props + const { numOwners, onReset, safe } = this.props return ( {(submitting: boolean, submitSucceeded: boolean) => ( @@ -84,7 +85,7 @@ class Threshold extends React.PureComponent { variant="raised" color="primary" onClick={submitSucceeded ? onReset : undefined} - type="submit" + type={submitSucceeded ? 'button' : 'submit'} disabled={submitting} > { submitSucceeded ? 'VISIT TXs' : 'FINISH' } From 5b4e0256f272f90b973f84136dc5c096e9875fac Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 6 Jun 2018 10:10:01 +0200 Subject: [PATCH 06/10] WA-235 adding fetchThreshold action --- src/routes/safe/store/actions/fetchThreshold.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/routes/safe/store/actions/fetchThreshold.js diff --git a/src/routes/safe/store/actions/fetchThreshold.js b/src/routes/safe/store/actions/fetchThreshold.js new file mode 100644 index 00000000..2ae28e69 --- /dev/null +++ b/src/routes/safe/store/actions/fetchThreshold.js @@ -0,0 +1,12 @@ +// @flow +import type { Dispatch as ReduxDispatch } from 'redux' +import { type GlobalState } from '~/store/index' +import { getSafeEthereumInstance } from '~/routes/safe/component/AddTransaction/createTransactions' +import updateThreshold from './updateThreshold' + +export default (safeAddress: string) => async (dispatch: ReduxDispatch) => { + const gnosisSafe = await getSafeEthereumInstance(safeAddress) + const actualThreshold = await gnosisSafe.getThreshold() + + return dispatch(updateThreshold(safeAddress, actualThreshold)) +} From b70b70c5df5a5d8b89bc5da4a9e045e3083c68ae Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 6 Jun 2018 10:11:52 +0200 Subject: [PATCH 07/10] WA-235 adding updateThreshold action --- .../safe/store/actions/updateThreshold.js | 19 ++++++++ .../safe/store/test/threshold.reducer.js | 48 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 src/routes/safe/store/actions/updateThreshold.js create mode 100644 src/routes/safe/store/test/threshold.reducer.js diff --git a/src/routes/safe/store/actions/updateThreshold.js b/src/routes/safe/store/actions/updateThreshold.js new file mode 100644 index 00000000..b24f41fd --- /dev/null +++ b/src/routes/safe/store/actions/updateThreshold.js @@ -0,0 +1,19 @@ +// @flow +import { createAction } from 'redux-actions' + +export const UPDATE_THRESHOLD = 'UPDATE_THRESHOLD' + +type ThresholdProps = { + safeAddress: string, + threshold: number, +} + +const updateDailyLimit = createAction( + UPDATE_THRESHOLD, + (safeAddress: string, threshold: number): ThresholdProps => ({ + safeAddress, + threshold: Number(threshold), + }), +) + +export default updateDailyLimit diff --git a/src/routes/safe/store/test/threshold.reducer.js b/src/routes/safe/store/test/threshold.reducer.js new file mode 100644 index 00000000..b1d483c3 --- /dev/null +++ b/src/routes/safe/store/test/threshold.reducer.js @@ -0,0 +1,48 @@ +// @flow +import { SAFE_REDUCER_ID } from '~/routes/safe/store/reducer/safe' +import { aNewStore } from '~/store' +import updateThreshold from '~/routes/safe/store/actions/updateThreshold' +import { aDeployedSafe } from './builder/deployedSafe.builder' + +const thresholdReducerTests = () => { + describe('Safe Actions[updateThreshold]', () => { + let store + beforeEach(async () => { + store = aNewStore() + }) + + it('reducer should return 3 when a safe of 3 threshold has just been created', async () => { + // GIVEN + const safeThreshold = 3 + const numOwners = 3 + + // WHEN + const safeAddress = await aDeployedSafe(store, 0.5, safeThreshold, numOwners) + + // THEN + const safes = store.getState()[SAFE_REDUCER_ID] + const threshold = safes.get(safeAddress).get('confirmations') + expect(threshold).not.toBe(undefined) + expect(threshold).toBe(safeThreshold) + }) + + it('reducer should change correctly', async () => { + // GIVEN + const safeThreshold = 3 + const numOwners = 3 + const safeAddress = await aDeployedSafe(store, 0.5, safeThreshold, numOwners) + + // WHEN + const newThreshold = 1 + await store.dispatch(updateThreshold(safeAddress, newThreshold)) + + // THEN + const safes = store.getState()[SAFE_REDUCER_ID] + const threshold = safes.get(safeAddress).get('confirmations') + expect(threshold).not.toBe(undefined) + expect(threshold).toBe(newThreshold) + }) + }) +} + +export default thresholdReducerTests From 13c1152c5b0cd3d9fce9ad0d24d162dededd2ab1 Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 6 Jun 2018 10:13:59 +0200 Subject: [PATCH 08/10] WA-235 Adding threshold test for checking store is updated correctly --- src/routes/safe/store/reducer/safe.js | 3 +++ src/routes/safe/store/test/safe.spec.js | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/routes/safe/store/reducer/safe.js b/src/routes/safe/store/reducer/safe.js index d430c49d..f8f64485 100644 --- a/src/routes/safe/store/reducer/safe.js +++ b/src/routes/safe/store/reducer/safe.js @@ -7,6 +7,7 @@ import { makeOwner } from '~/routes/safe/store/model/owner' import { type Safe, makeSafe } from '~/routes/safe/store/model/safe' import { load, saveSafes, SAFES_KEY } from '~/utils/localStorage' import { makeDailyLimit } from '~/routes/safe/store/model/dailyLimit' +import updateThreshold, { UPDATE_THRESHOLD } from '~/routes/safe/store/actions/updateThreshold' export const SAFE_REDUCER_ID = 'safes' @@ -50,4 +51,6 @@ export default handleActions({ }, [UPDATE_DAILY_LIMIT]: (state: State, action: ActionType): State => state.updateIn([action.payload.safeAddress, 'dailyLimit'], () => makeDailyLimit(action.payload.dailyLimit)), + [UPDATE_THRESHOLD]: (state: State, action: ActionType): State => + state.updateIn([action.payload.safeAddress, 'confirmations'], () => action.payload.threshold), }, Map()) diff --git a/src/routes/safe/store/test/safe.spec.js b/src/routes/safe/store/test/safe.spec.js index 3c431b4b..8a0ebd4d 100644 --- a/src/routes/safe/store/test/safe.spec.js +++ b/src/routes/safe/store/test/safe.spec.js @@ -2,6 +2,7 @@ import balanceReducerTests from './balance.reducer' import safeReducerTests from './safe.reducer' import dailyLimitReducerTests from './dailyLimit.reducer' +import thresholdReducerTests from './threshold.reducer' import balanceSelectorTests from './balance.selector' import safeSelectorTests from './safe.selector' import grantedSelectorTests from './granted.selector' @@ -13,6 +14,7 @@ describe('Safe Test suite', () => { safeReducerTests() balanceReducerTests() dailyLimitReducerTests() + thresholdReducerTests() // SAFE SELECTOR safeSelectorTests() From 31e5833b8eadeba2d00c06dba9dc237b3d871626 Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 6 Jun 2018 10:43:56 +0200 Subject: [PATCH 09/10] WA-238 Updating UI after processing TX for changing threshold --- src/routes/safe/component/Threshold/actions.js | 16 ++++++++++++++++ src/routes/safe/component/Threshold/index.jsx | 8 +++++--- .../safe/component/Transactions/actions.js | 8 +++++++- src/routes/safe/component/Transactions/index.jsx | 6 +++++- 4 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 src/routes/safe/component/Threshold/actions.js diff --git a/src/routes/safe/component/Threshold/actions.js b/src/routes/safe/component/Threshold/actions.js new file mode 100644 index 00000000..e0b10a5a --- /dev/null +++ b/src/routes/safe/component/Threshold/actions.js @@ -0,0 +1,16 @@ +// @flow +import fetchThreshold from '~/routes/safe/store/actions/fetchThreshold' +import fetchTransactions from '~/routes/safe/store/actions/fetchTransactions' + +type FetchThreshold = typeof fetchThreshold +type FetchTransactions = typeof fetchTransactions + +export type Actions = { + fetchThreshold: FetchThreshold, + fetchTransactions: FetchTransactions, +} + +export default { + fetchThreshold, + fetchTransactions, +} diff --git a/src/routes/safe/component/Threshold/index.jsx b/src/routes/safe/component/Threshold/index.jsx index 61c4c565..b2b66b66 100644 --- a/src/routes/safe/component/Threshold/index.jsx +++ b/src/routes/safe/component/Threshold/index.jsx @@ -13,8 +13,9 @@ import { composeValidators, minValue, maxValue, mustBeInteger, required } from ' import { getSafeEthereumInstance, createTransaction } from '~/routes/safe/component/AddTransaction/createTransactions' import { sleep } from '~/utils/timer' import selector, { type SelectorProps } from './selector' +import actions, { type Actions } from './actions' -type Props = SelectorProps & { +type Props = SelectorProps & Actions & { numOwners: number, safe: Safe, onReset: () => void, @@ -65,7 +66,8 @@ class Threshold extends React.PureComponent { const data = gnosisSafe.contract.changeThreshold.getData(newThreshold) await createTransaction(safe, `Change Safe's threshold [${nonce}]`, safe.get('address'), 0, nonce, userAddress, data) await sleep(1500) - // this.props.fetchThreshold(safe.get('address')) + this.props.fetchTransactions() + this.props.fetchThreshold(safe.get('address')) } render() { @@ -98,4 +100,4 @@ class Threshold extends React.PureComponent { } } -export default connect(selector)(Threshold) +export default connect(selector, actions)(Threshold) diff --git a/src/routes/safe/component/Transactions/actions.js b/src/routes/safe/component/Transactions/actions.js index 681ad469..e0b10a5a 100644 --- a/src/routes/safe/component/Transactions/actions.js +++ b/src/routes/safe/component/Transactions/actions.js @@ -1,10 +1,16 @@ // @flow +import fetchThreshold from '~/routes/safe/store/actions/fetchThreshold' import fetchTransactions from '~/routes/safe/store/actions/fetchTransactions' +type FetchThreshold = typeof fetchThreshold +type FetchTransactions = typeof fetchTransactions + export type Actions = { - fetchTransactions: typeof fetchTransactions, + fetchThreshold: FetchThreshold, + fetchTransactions: FetchTransactions, } export default { + fetchThreshold, fetchTransactions, } diff --git a/src/routes/safe/component/Transactions/index.jsx b/src/routes/safe/component/Transactions/index.jsx index 5dac8846..e8a22e87 100644 --- a/src/routes/safe/component/Transactions/index.jsx +++ b/src/routes/safe/component/Transactions/index.jsx @@ -17,10 +17,14 @@ type Props = SelectorProps & Actions & { } class Transactions extends React.Component { onProcessTx = async (tx: Transaction, alreadyConfirmed: number) => { - const { fetchTransactions, safeAddress, userAddress } = this.props + const { + fetchTransactions, safeAddress, userAddress, fetchThreshold, + } = this.props + await processTransaction(safeAddress, tx, alreadyConfirmed, userAddress) await sleep(1200) fetchTransactions() + fetchThreshold(safeAddress) } render() { From 058c6e04fd48b059d082789ab36b42116247e99c Mon Sep 17 00:00:00 2001 From: apanizo Date: Wed, 6 Jun 2018 11:26:45 +0200 Subject: [PATCH 10/10] WA-235 Fix tests - Adapting buttons' index based on the new layout --- .../safe/test/Safe.multisig.1owners1threshold.test.js | 2 +- src/routes/safe/test/Safe.withdrawn.test.js | 6 +++--- src/routes/safe/test/testMultisig.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/routes/safe/test/Safe.multisig.1owners1threshold.test.js b/src/routes/safe/test/Safe.multisig.1owners1threshold.test.js index 1567f58c..80d8b697 100644 --- a/src/routes/safe/test/Safe.multisig.1owners1threshold.test.js +++ b/src/routes/safe/test/Safe.multisig.1owners1threshold.test.js @@ -45,7 +45,7 @@ describe('React DOM TESTS > Withdrawn funds from safe', () => { // $FlowFixMe const buttons = TestUtils.scryRenderedComponentsWithType(Safe, Button) - const addTxButton = buttons[1] + const addTxButton = buttons[2] expect(addTxButton.props.children).toEqual(ADD_MULTISIG_BUTTON_TEXT) await sleep(1800) // Give time to enable Add button TestUtils.Simulate.click(TestUtils.scryRenderedDOMComponentsWithTag(addTxButton, 'button')[0]) diff --git a/src/routes/safe/test/Safe.withdrawn.test.js b/src/routes/safe/test/Safe.withdrawn.test.js index a0803c57..a985b3cc 100644 --- a/src/routes/safe/test/Safe.withdrawn.test.js +++ b/src/routes/safe/test/Safe.withdrawn.test.js @@ -46,7 +46,7 @@ describe('React DOM TESTS > Withdrawn funds from safe', () => { // $FlowFixMe const buttons = TestUtils.scryRenderedComponentsWithType(Safe, Button) - const withdrawnButton = buttons[0] + const withdrawnButton = buttons[1] expect(withdrawnButton.props.children).toEqual(WITHDRAWN_BUTTON_TEXT) TestUtils.Simulate.click(TestUtils.scryRenderedDOMComponentsWithTag(withdrawnButton, 'button')[0]) await sleep(4000) @@ -96,7 +96,7 @@ describe('React DOM TESTS > Withdrawn funds from safe', () => { const Safe = TestUtils.findRenderedComponentWithType(SafeDom, SafeView) // $FlowFixMe const buttons = TestUtils.scryRenderedComponentsWithType(Safe, Button) - const addTxButton = buttons[1] + const addTxButton = buttons[2] expect(addTxButton.props.children).toEqual(ADD_MULTISIG_BUTTON_TEXT) expect(addTxButton.props.disabled).toBe(true) @@ -110,7 +110,7 @@ describe('React DOM TESTS > Withdrawn funds from safe', () => { const Safe = TestUtils.findRenderedComponentWithType(SafeDom, SafeView) // $FlowFixMe const buttons = TestUtils.scryRenderedComponentsWithType(Safe, Button) - const addTxButton = buttons[0] + const addTxButton = buttons[1] expect(addTxButton.props.children).toEqual(WITHDRAWN_BUTTON_TEXT) expect(addTxButton.props.disabled).toBe(true) diff --git a/src/routes/safe/test/testMultisig.js b/src/routes/safe/test/testMultisig.js index c4264cd5..272ce34d 100644 --- a/src/routes/safe/test/testMultisig.js +++ b/src/routes/safe/test/testMultisig.js @@ -43,7 +43,7 @@ export const addFundsTo = async (SafeDom, destination: string) => { // $FlowFixMe const buttons = TestUtils.scryRenderedComponentsWithType(Safe, Button) - const addTxButton = buttons[1] + const addTxButton = buttons[2] expect(addTxButton.props.children).toEqual(ADD_MULTISIG_BUTTON_TEXT) await sleep(1800) // Give time to enable Add button TestUtils.Simulate.click(TestUtils.scryRenderedDOMComponentsWithTag(addTxButton, 'button')[0]) @@ -54,7 +54,7 @@ export const listTxsOf = (SafeDom) => { // $FlowFixMe const buttons = TestUtils.scryRenderedComponentsWithType(Safe, Button) - const seeTx = buttons[2] + const seeTx = buttons[3] expect(seeTx.props.children).toEqual(SEE_MULTISIG_BUTTON_TEXT) TestUtils.Simulate.click(TestUtils.scryRenderedDOMComponentsWithTag(seeTx, 'button')[0]) }