WA-238 Adding logic (including tests) for handling change threshold

config/jest/jest.setup.js

@@ -1,2 +1,2 @@
 // @flow
-jest.setTimeout(30000)
+jest.setTimeout(45000)

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<Confirmation> = 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<Confirmation> = 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)
 }

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<Confirmation> = 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<string, List<Transaction>> = 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<Confirmation> = 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<Confirmation> = 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<string, List<Transaction>> = 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, 0, firstTxName, firstNonce, value, 2, destination, '0x', '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, 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<Confirmation> = 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<Confirmation> = 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<string, List<Transaction>> = loadSafeTransactions()
@@ -112,7 +112,7 @@ describe('Transactions Suite', () => {
     const txConfirmations: List<Confirmation> = 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<Confirmation> = 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<Confirmation> = 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<string, List<Transaction>> = loadSafeTransactions()
@@ -185,7 +185,7 @@ describe('Transactions Suite', () => {
     const nonce: number = 10
     const tx = ''
     const confirmations: List<Confirmation> = 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<Confirmation> = 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<string, List<Transaction>> = loadSafeTransactions()

src/routes/safe/component/AddTransaction/test/transactionsHelper.js

@@ -20,7 +20,7 @@ export const testSizeOfTransactions = (safeTxs: List<Transaction> | typeof undef
 export const testTransactionFrom = (
   safeTxs: List<Transaction> | 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<Confirmation> = tx.get('confirmations')
   const firstConfirmation: Confirmation | typeof undefined = confirmations.get(0)

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 execTransaction(gnosisSafe, txDestination, txValue, nonce, userAddress, data)
-    : await execConfirmation(gnosisSafe, txDestination, txValue, nonce, userAddress)
+    : await execConfirmation(gnosisSafe, txDestination, txValue, nonce, userAddress, data)
 
   checkReceiptStatus(txHash)
 
@@ -130,5 +131,6 @@ export const processTransaction = async (
     thresholdReached ? txHash : '',
     safeAddress,
     threshold,
+    data,
   )
 }

src/routes/safe/store/model/transaction.js

@@ -11,6 +11,7 @@ export type TransactionProps = {
   confirmations: List<Confirmation>,
   destination: string,
   tx: string,
+  data: string,
 }
 
 export const makeTransaction: RecordFactory<TransactionProps> = Record({
@@ -21,6 +22,7 @@ export const makeTransaction: RecordFactory<TransactionProps> = Record({
   destination: '',
   tx: '',
   threshold: 0,
+  data: '',
 })
 
 export type Transaction = RecordOf<TransactionProps>

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)
+  })
+})