Merge pull request #60 from gnosis/development

Development

.github/workflows/e2e-test.yml

@@ -0,0 +1,26 @@
+name: e2e Test
+on:
+  push:
+    branches:
+      - development
+      - main
+  pull_request:
+    branches:
+      - development
+      - main
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        node-version: [14.x]
+    steps:
+      - uses: actions/checkout@v2
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v1
+        with:
+          node-version: ${{ matrix.node-version }}
+      - run: |
+          yarn install
+          yarn build
+          yarn test:ci

.github/workflows/test.yml

@@ -12,6 +12,7 @@ jobs:
         uses: actions/setup-node@v1
         with:
           node-version: ${{ matrix.node-version }}
-      - run: yarn install
-      - run: yarn build
-      - run: yarn test
+      - run: |
+          yarn install
+          yarn build
+          yarn test

package.json

@@ -5,7 +5,8 @@
     "clean": "lerna clean",
     "unbuild": "lerna run unbuild",
     "build": "lerna run build --stream",
-    "test": "FORCE_COLOR=1 lerna run test --stream"
+    "test": "FORCE_COLOR=1 lerna run test --stream",
+    "test:ci": "FORCE_COLOR=1 lerna run test:ci --stream"
   },
   "workspaces": {
     "packages": [

packages/safe-core-sdk/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@gnosis.pm/safe-core-sdk",
-  "version": "0.3.1",
+  "version": "0.4.0",
   "description": "Safe Core SDK",
   "main": "dist/src/index.js",
   "typings": "dist/src/index.d.ts",

packages/safe-core-sdk/src/Safe.ts

@@ -14,7 +14,11 @@ import { sameString } from './utils'
 import { generatePreValidatedSignature, generateSignature } from './utils/signatures'
 import { estimateGasForTransactionExecution } from './utils/transactions/gas'
 import EthSafeTransaction from './utils/transactions/SafeTransaction'
-import { TransactionOptions, TransactionResult } from './utils/transactions/types'
+import {
+  CallTransactionOptionalProps,
+  TransactionOptions,
+  TransactionResult
+} from './utils/transactions/types'
 import {
   encodeMultiSendData,
   standardizeMetaTransactionData,
@@ -39,6 +43,27 @@ export interface ConnectSafeConfig {
   contractNetworks?: ContractNetworksConfig
 }
 
+export interface AddOwnerTxParams {
+  /** ownerAddress - The address of the new owner */
+  ownerAddress: string
+  /** threshold - The new threshold */
+  threshold?: number
+}
+
+export interface RemoveOwnerTxParams {
+  /** ownerAddress - The address of the owner that will be removed */
+  ownerAddress: string
+  /** threshold - The new threshold */
+  threshold?: number
+}
+
+export interface SwapOwnerTxParams {
+  /** oldOwnerAddress - The old owner address */
+  oldOwnerAddress: string
+  /** newOwnerAddress - The new owner address */
+  newOwnerAddress: string
+}
+
 class Safe {
   #ethAdapter!: EthAdapter
   #contractManager!: ContractManager
@@ -336,15 +361,20 @@ class Safe {
    * Returns the Safe transaction to enable a Safe module.
    *
    * @param moduleAddress - The desired module address
+   * @param options - The transaction optional properties
    * @returns The Safe transaction ready to be signed
    * @throws "Invalid module address provided"
    * @throws "Module provided is already enabled"
    */
-  async getEnableModuleTx(moduleAddress: string): Promise<SafeTransaction> {
+  async getEnableModuleTx(
+    moduleAddress: string,
+    options?: CallTransactionOptionalProps
+  ): Promise<SafeTransaction> {
     const safeTransaction = await this.createTransaction({
       to: this.getAddress(),
       value: '0',
-      data: await this.#moduleManager.encodeEnableModuleData(moduleAddress)
+      data: await this.#moduleManager.encodeEnableModuleData(moduleAddress),
+      ...options
     })
     return safeTransaction
   }
@@ -353,75 +383,92 @@ class Safe {
    * Returns the Safe transaction to disable a Safe module.
    *
    * @param moduleAddress - The desired module address
+   * @param options - The transaction optional properties
    * @returns The Safe transaction ready to be signed
    * @throws "Invalid module address provided"
    * @throws "Module provided is not enabled already"
    */
-  async getDisableModuleTx(moduleAddress: string): Promise<SafeTransaction> {
+  async getDisableModuleTx(
+    moduleAddress: string,
+    options?: CallTransactionOptionalProps
+  ): Promise<SafeTransaction> {
     const safeTransaction = await this.createTransaction({
       to: this.getAddress(),
       value: '0',
-      data: await this.#moduleManager.encodeDisableModuleData(moduleAddress)
+      data: await this.#moduleManager.encodeDisableModuleData(moduleAddress),
+      ...options
     })
     return safeTransaction
   }
 
   /**
    * Returns the Safe transaction to add an owner and optionally change the threshold.
    *
-   * @param ownerAddress - The address of the new owner
-   * @param threshold - The new threshold
+   * @param params - The transaction params
+   * @param options - The transaction optional properties
    * @returns The Safe transaction ready to be signed
    * @throws "Invalid owner address provided"
    * @throws "Address provided is already an owner"
    * @throws "Threshold needs to be greater than 0"
    * @throws "Threshold cannot exceed owner count"
    */
-  async getAddOwnerTx(ownerAddress: string, threshold?: number): Promise<SafeTransaction> {
+  async getAddOwnerTx(
+    { ownerAddress, threshold }: AddOwnerTxParams,
+    options?: CallTransactionOptionalProps
+  ): Promise<SafeTransaction> {
     const safeTransaction = await this.createTransaction({
       to: this.getAddress(),
       value: '0',
-      data: await this.#ownerManager.encodeAddOwnerWithThresholdData(ownerAddress, threshold)
+      data: await this.#ownerManager.encodeAddOwnerWithThresholdData(ownerAddress, threshold),
+      ...options
     })
     return safeTransaction
   }
 
   /**
    * Returns the Safe transaction to remove an owner and optionally change the threshold.
    *
-   * @param ownerAddress - The address of the owner that will be removed
-   * @param threshold - The new threshold
+   * @param params - The transaction params
+   * @param options - The transaction optional properties
    * @returns The Safe transaction ready to be signed
    * @throws "Invalid owner address provided"
    * @throws "Address provided is not an owner"
    * @throws "Threshold needs to be greater than 0"
    * @throws "Threshold cannot exceed owner count"
    */
-  async getRemoveOwnerTx(ownerAddress: string, threshold?: number): Promise<SafeTransaction> {
+  async getRemoveOwnerTx(
+    { ownerAddress, threshold }: RemoveOwnerTxParams,
+    options?: CallTransactionOptionalProps
+  ): Promise<SafeTransaction> {
     const safeTransaction = await this.createTransaction({
       to: this.getAddress(),
       value: '0',
-      data: await this.#ownerManager.encodeRemoveOwnerData(ownerAddress, threshold)
+      data: await this.#ownerManager.encodeRemoveOwnerData(ownerAddress, threshold),
+      ...options
     })
     return safeTransaction
   }
 
   /**
    * Returns the Safe transaction to replace an owner of the Safe with a new one.
    *
-   * @param oldOwnerAddress - The old owner address
-   * @param newOwnerAddress - The new owner address
+   * @param params - The transaction params
+   * @param options - The transaction optional properties
    * @returns The Safe transaction ready to be signed
    * @throws "Invalid new owner address provided"
    * @throws "Invalid old owner address provided"
    * @throws "New address provided is already an owner"
    * @throws "Old address provided is not an owner"
    */
-  async getSwapOwnerTx(oldOwnerAddress: string, newOwnerAddress: string): Promise<SafeTransaction> {
+  async getSwapOwnerTx(
+    { oldOwnerAddress, newOwnerAddress }: SwapOwnerTxParams,
+    options?: CallTransactionOptionalProps
+  ): Promise<SafeTransaction> {
     const safeTransaction = await this.createTransaction({
       to: this.getAddress(),
       value: '0',
-      data: await this.#ownerManager.encodeSwapOwnerData(oldOwnerAddress, newOwnerAddress)
+      data: await this.#ownerManager.encodeSwapOwnerData(oldOwnerAddress, newOwnerAddress),
+      ...options
     })
     return safeTransaction
   }
@@ -430,15 +477,20 @@ class Safe {
    * Returns the Safe transaction to change the threshold.
    *
    * @param threshold - The new threshold
+   * @param options - The transaction optional properties
    * @returns The Safe transaction ready to be signed
    * @throws "Threshold needs to be greater than 0"
    * @throws "Threshold cannot exceed owner count"
    */
-  async getChangeThresholdTx(threshold: number): Promise<SafeTransaction> {
+  async getChangeThresholdTx(
+    threshold: number,
+    options?: CallTransactionOptionalProps
+  ): Promise<SafeTransaction> {
     const safeTransaction = await this.createTransaction({
       to: this.getAddress(),
       value: '0',
-      data: await this.#ownerManager.encodeChangeThresholdData(threshold)
+      data: await this.#ownerManager.encodeChangeThresholdData(threshold),
+      ...options
     })
     return safeTransaction
   }
@@ -477,6 +529,14 @@ class Safe {
       )
     }
 
+    const value = BigNumber.from(safeTransaction.data.value)
+    if (!value.isZero()) {
+      const balance = await this.getBalance()
+      if (value.gt(BigNumber.from(balance))) {
+        throw new Error('Not enough Ether funds')
+      }
+    }
+
     const gasLimit =
       options?.gasLimit ||
       (await estimateGasForTransactionExecution(

packages/safe-core-sdk/src/index.ts

@@ -1,14 +1,24 @@
 import { ContractNetworksConfig } from './configuration/contracts'
-import EthAdapter from './ethereumLibs/EthAdapter'
+import EthAdapter, { EthAdapterTransaction } from './ethereumLibs/EthAdapter'
 import EthersAdapter, { EthersAdapterConfig } from './ethereumLibs/EthersAdapter'
 import Web3Adapter, { Web3AdapterConfig } from './ethereumLibs/Web3Adapter'
-import Safe, { ConnectSafeConfig, SafeConfig } from './Safe'
+import Safe, {
+  AddOwnerTxParams,
+  ConnectSafeConfig,
+  RemoveOwnerTxParams,
+  SafeConfig,
+  SwapOwnerTxParams
+} from './Safe'
 import SafeFactory, {
   SafeAccountConfig,
   SafeDeploymentConfig,
   SafeFactoryConfig
 } from './safeFactory'
-import { TransactionOptions, TransactionResult } from './utils/transactions/types'
+import {
+  CallTransactionOptionalProps,
+  TransactionOptions,
+  TransactionResult
+} from './utils/transactions/types'
 
 export default Safe
 export {
@@ -17,6 +27,7 @@ export {
   SafeAccountConfig,
   SafeDeploymentConfig,
   EthAdapter,
+  EthAdapterTransaction,
   Web3AdapterConfig,
   Web3Adapter,
   EthersAdapterConfig,
@@ -25,5 +36,9 @@ export {
   ConnectSafeConfig,
   ContractNetworksConfig,
   TransactionOptions,
-  TransactionResult
+  TransactionResult,
+  CallTransactionOptionalProps,
+  AddOwnerTxParams,
+  RemoveOwnerTxParams,
+  SwapOwnerTxParams
 }

packages/safe-core-sdk/src/utils/transactions/types.ts

@@ -1,6 +1,19 @@
 import { ContractTransaction } from '@ethersproject/contracts'
+import { OperationType } from '@gnosis.pm/safe-core-sdk-types'
 import { PromiEvent, TransactionReceipt } from 'web3-core/types'
 
+interface SafeTransactionOptionalProps {
+  operation?: OperationType
+  safeTxGas?: number
+  baseGas?: number
+  gasPrice?: number
+  gasToken?: string
+  refundReceiver?: string
+  nonce?: number
+}
+
+export type CallTransactionOptionalProps = Omit<SafeTransactionOptionalProps, 'operation'>
+
 export interface TransactionOptions {
   from?: string
   gas?: number | string

packages/safe-core-sdk/src/utils/transactions/utils.ts

@@ -1,5 +1,5 @@
 import { arrayify } from '@ethersproject/bytes'
-import { pack } from '@ethersproject/solidity'
+import { pack as solidityPack } from '@ethersproject/solidity'
 import {
   MetaTransactionData,
   OperationType,
@@ -57,7 +57,7 @@ export async function standardizeSafeTransactionData(
 
 function encodeMetaTransaction(tx: MetaTransactionData): string {
   const data = arrayify(tx.data)
-  const encoded = pack(
+  const encoded = solidityPack(
     ['uint8', 'address', 'uint256', 'uint256', 'bytes'],
     [tx.operation, tx.to, tx.value, data.length, data]
   )

packages/safe-core-sdk/tests/execution.test.ts

@@ -38,6 +38,28 @@ describe('Transactions execution', () => {
   })
 
   describe('executeTransaction', async () => {
+    it('should fail if there are not enough Ether funds', async () => {
+      const { accounts, contractNetworks } = await setupTests()
+      const [account1, account2] = accounts
+      const safe = await getSafeWithOwners([account1.address])
+      const ethAdapter = await getEthAdapter(account1.signer)
+      const safeSdk1 = await Safe.create({
+        ethAdapter,
+        safeAddress: safe.address,
+        contractNetworks
+      })
+      const safeInitialBalance = await safeSdk1.getBalance()
+      chai.expect(safeInitialBalance.toString()).to.be.eq('0')
+      const tx = await safeSdk1.createTransaction({
+        to: account2.address,
+        value: '500000000000000000', // 0.5 ETH
+        data: '0x'
+      })
+      await chai
+        .expect(safeSdk1.executeTransaction(tx))
+        .to.be.rejectedWith('Not enough Ether funds')
+    })
+
     it('should fail if there are not enough signatures (1 missing)', async () => {
       const { accounts, contractNetworks } = await setupTests()
       const [account1, account2, account3] = accounts