The project should be properly tested. For this we should start with testing the mapping layer

Tasks

• [ ] Test multisig tx mapping
• Test ethereum tx mapping
• Test module tx mapping
• Test unknown mapping

Notes

• https://stackoverflow.com/questions/38995892/how-to-move-tests-into-a-separate-file-for-binaries-in-rusts-cargo
• https://doc.rust-lang.org/book/ch11-03-test-organization.html#unit-tests

It would be nice if we can generate api documentation for the existing endpoints

Notes/ References

• rwf2/Rocket#297
• https://github.com/GREsau/okapi
• https://doc.rust-lang.org/rustdoc/what-is-rustdoc.html
• There is a PR open rwf2/Rocket#1624 with activity as early as a couple of days back

Write classes to generate deserialisers for all the possible types supplied by the transaction service /safe/{address}/all-transactions

TxType can be { ETHEREUM_TRANSACTION, MULTISIG_TRANSACTION, MODULE_TRANSACTION }

https://safe-transaction.staging.gnosisdev.com/

Investigate: https://github.com/dtolnay/typetag for polymorphic deserialization

Preconditions

• The SafeInfo is fetched and cached (needed for status calculation) #28
• The Tokens endpoint is fetched and cached (to serve the correct info for both historic and pending transactions) #29

Is your feature request related to a problem? Please describe.
Multisig transactions should be mapped into all domain types as defined in #24. Additionally, the status of the transaction should be calculated as in the TransactionRepository in Android for now.

Describe the solution you'd like
The detection of transaction type is already done (like in Android) by observing the dataDecoded field. However, in certain cases we have erroneous classifications, as certain ERC721 tokens call methods with the same signature as ERC20 for transfers. Therefore the heuristic should be implemented to match that of iOS.
The safe and tokenInfo should be fetched and cached for each transaction (beyond the scope of this issue, see preconditions ☝️ )

Additional context
For reference, the classes described in #24 will be used. For mapping logic the TransactionRepository implementation will be used and filling out of data will be done as needed.

Describe the solution you'd like
Write a version of the multisig android app that consumes the API for rendering the transactions list. The result should be presented as a link to a branch in the gnosis/safe-android repo .

Additional context
The android app, just like the iOS app, extensively maps the backend types into the domain types. This duplication is error prone. This will be a first PoC to establish the usefulness of the client gateway.

• Check endpoint naming #47
• Check project structure (e.g. modules)
• Check warnings
• Convert prints to logs #50

The project should be continiously build when changes are done.

Tasks

• Add CI (check if travis works with rust)
• Run tests (required tests e.g. #20 )

Notes

• https://docs.travis-ci.com/user/languages/rust/

Default project should be https://github.com/orgs/gnosis/projects/4 if possible

Notes

• Docs https://docs.github.com/en/github/building-a-strong-community/configuring-issue-templates-for-your-repository
• Example https://github.com/gnosis/safe-contracts/tree/development/.github/ISSUE_TEMPLATE

Describe the bug
We get a 500 when the TokenInfo struct is null in a Transfer struct from the backend.

To Reproduce
Steps to reproduce the behavior:

• Load a 2nd page of data for the safe in the path displayed here: transactions/0x1230B3d59858296A31053C1b8562Ecf89A2f888b?next=limit%3D20%26offset%3D20

Expected behavior
We should return make TokenInfo nullable for Transfers as the /tokens/{address} endpoint probably won't contain these tokens anyhow.

• Example of transaction list with different types
• Examples of different detail responses

Describe the bug
Status of the transaction is changed with the delay of more than 10 min from awaiting execution to execute.
The same was with changing statuses from awaiting confirmation to awaiting execution - the status was updated when the new transaction was created

To Reproduce
Steps to reproduce the behavior:

1. Create a transaction in the safe with a few confirmation

2. Open mobile app

3. Check transactions list
   Current result: The transaction in the Queue appears

4. Confirm transaction without executing

5. Wait til the status is updated in the web app

6. Refresh the page ( pull to refresh, switch safes, go to another page and come back, restart the app)
   Current result: The transaction status is not changed for a long time.

Expected behavior
The status on the client should be changed as soon as it's changed on the back-end

Desktop (please complete the following information):

• OS: android
• Version 0.2.0

Additional context
Add any other context about the problem here.

It should be possible to submit a multisig transaction for execution using the information provided by the details endpoint.

See https://docs.google.com/document/d/1Bu9-CM9Fy-9duNqb6h6MlgzTE4oxFHDyCGAfBitIF6M/edit#

Note: For now use 0.5.0-dev

ToDos:

• Upgrade rocket (https://rocket.rs/master/guide/getting-started/)
• Migrate redis
• Convert all routes to async
• Use async reqwest client
• Create separate env for testing

References:

• Redis support: rwf2/Rocket#1471
• Stable rust support: rwf2/Rocket#19

Tasks

• Add redis to service
• Add possibility to enable simple caching (set/ get cache by key with timeout)
• Add possibility to invalidate cache (by key)

Bug description
The successfully executed transaction ( it has status success in webapp and the receiver got the tokens) is marked as cancelled in the mobile app
"isExecuted": true,
"isSuccessful": true,
Steps To Reproduce

1. Load safe with a few owners ( all owners or at least 2 of them should confirm transaction)
2. Send funds in the web app in the same safe
3. Confirm and execute the transaction

Current result:
The cancelled transaction appears in history. After page reloading, the cancelled transaction is replaced with successful with new execution time

Expected Result
It's confusing to see the same transaction with two different states.
Why cancelled transaction appears?

Screenshots
Look at a transaction with nonce 53 ( pay attantion to the date and time )

The same transaction in a few sec after page updating:

Device & App version

• Device/OS: iPhone X/13.15.1
• App version: 0.5.0 (61)

Additional info
(Optional)

Currently it is quite tedious to extract params from DataDecode with fallbacks (e.g. to and _to). It would be nice to have a easy way to handle this.

Note

It should be safer to use the position of the argument rather than the name of the argument.

Todo

• add reference to what is displayed on mobile
• create issues for subtasks

Tasks

• Map MultiSig transaction #27
• Map Ethereum transactions #26
• Map Module transactions #25
• Write demo branch in the gnosis/safe-android project consuming the API #44

@liliya-soroka commented on Thu Aug 06 2020

Describe the bug
Custom transactions are displayed for some of the ERC20, ERC721, ETH outgoing tx
Actual for the transactions in waiting for confirmation and executed statuses

To Reproduce
Steps to reproduce the behavior:

ERC20 and ERC721:

1. Go to the safe 0x1230B3d59858296A31053C1b8562Ecf89A2f888b
2. Open transactions details
3. Check txs with nonce 176,175,174

ETH:

1. Go to the safe 0x938bae50a210b80EA233112800Cd5Bc2e7644300
2. Open transactions details
3. Check txs with 46

Expected behavior
If it's outgoing ERC20 or ERC721 tx it should be displayed as outgoing tx ( not custom)

Screenshots
in android app

on web

Smartphone (please complete the following information):

• Device: Huawei P30
• Android Version: 10
• App Version 1.12.1-79

Additional context
Add any other context about the problem here.

Tasks

• Add docker support
• Check requirements by devops for deployment

Notes

• Configure redis (see #17)

• Add webhook target with auth token
• Invalidate cache with pattern
• Invalidate cache for Safe when webhook is triggered

References

• Webhook payloads: https://github.com/gnosis/safe-transaction-service/blob/2cf8365eb7af8a01b1012515ffe0e304d532741e/safe_transaction_service/history/signals.py

• fetch usd prices from safe-transaction service
• use https://exchangeratesapi.io/ to fetch other exchange rates
• combine to return requested exchange rate

1. Transaction summary list endpoint (/v1/safes/<safe-address>/transactions/?next=):

• Android integrated safe-global/safe-android#776
• iOS usable

2. Transaction details endpoint (/v1/transactions/<transaction_id>):

• Android integrated safe-global/safe-android#788
• iOS usable

For each of the above tasks it should be checked also with iOS that the returned information is sufficient and can be used by them.

• Generate test coverage
• Upload test coverage

Open questions

• What service to use

References

• https://github.com/gnosis/dex-services/blob/master/.travis.yml#L62
  • https://coveralls.io/github/gnosis/dex-services
  • Note from @e00E: I think there are some places where the coverage report wrongly reports lines or branches not being covered but overall it works if the goal isn't to get 100% coverage.
• gnosis/dex-services#688

Describe the bug
Sometimes Parameters.value can be either a string or an array. The field is named the same, just the type changes.

"parameters": [
            {
                "name": "_owners",
                "type": "address[]",
                "value": [
                    "0x65F8236309e5A99Ff0d129d04E486EBCE20DC7B0",
                    "0xF2CeA96575d6b10f51d9aF3b10e3e4E5738aa6bd"
                ]
            },
            {
                "name": "_threshold",
                "type": "uint256",
                "value": "2"
            },
//...

To Reproduce

• Add the data_decoded field to the CreationTransaction struct for the backend
• Make a request to /safe/0x1230B3d59858296A31053C1b8562Ecf89A2f888b/creation to the transaction service, try deserialisation 🔥

Expected behaviour
The service should not crash and handle the case when Parameters have a flat value or a vector seamlessly.

Is your feature request related to a problem? Please describe.
The clients currently deserialise the polymorphic response from the /safe/{address}/all-transactions into Multisig, Module and Ethereum transactions. Then map each of those types into Transfer, Settings or Custom transaction types.

Describe the solution you'd like
Deserialization is already largely implemented. The domain types (Transfer, Settings and Custom) should be defined according to the needs of the apps.

Additional context
For reference, the domain transaction data layouts expected by Android are as follow:

sealed class Transaction {
    abstract val status: TransactionStatus

    // If status is Successful, Failed or Canceled, the confirmations can be null
    abstract val confirmations: Int?

    data class Custom(
        override val status: TransactionStatus,
        override val confirmations: Int?,
        val nonce: BigInteger?,
        val address: Solidity.Address,
        val dataSize: Long,
        val date: String?,
        val value: BigInteger
    ) : Transaction()

    data class SettingsChange(
        override val status: TransactionStatus,
        override val confirmations: Int?,
        val dataDecoded: DataDecodedDto,
        val date: String?,
        val nonce: BigInteger
    ) : Transaction()

    data class Transfer(
        override val status: TransactionStatus,
        override val confirmations: Int?,
        val recipient: Solidity.Address,
        val sender: Solidity.Address,
        val value: BigInteger,
        val date: String?,
        val tokenInfo: ServiceTokenInfo?,
        val nonce: BigInteger?
    ) : Transaction()
}

enum class TransactionStatus() {
    AwaitingConfirmations,
    AwaitingExecution,
    Cancelled,
    Failed,
    Success,
    Pending // Not supported yet as these correspond to the ones issued from the device
}

Is your feature request related to a problem? Please describe.
Multisig Transaction classification is a bit confusing and could be refactored. Specifically, there is a token_info request on every classification, which doesn't make sense in 60% of the cases (in the unit tests we just mock it as an error reponse). These cases are Transfer::Ether, SettingsChange and Custom transaction.

Describe the solution you'd like
The solution should prevent unnecessary requests to the InfoProvider even though we have a cache, we are not able to distinguish an error that matters from an error that happens when the request doesn't make sense given the context.

Describe alternatives you've considered
Switching the order of checks so that we only request token information when we strictly need it. However, the order in which we check could matter. First, we should cover with unit tests before commencing this refactor.

Make the deserialised response of Transactions from the backend, serialised into a differently, but also polymorphic, list of Transactions

Currently we only display name, type and value, but the transaction service also returns dataDecoded if it possible. This should be forwarded to the clients, so that they can display it.

It would be helpful for the clients to have a field incoming in the transfers. We need this field to render the transfer in the tx list and tx details.

The rules to determine incoming and outgoing status is described in this document:
https://docs.google.com/document/d/1vhTu5NbB-6m8nE3cqTS3bRONigPO3NA5YkJqNTA6AOk/edit

Notes:
Related bug: #46

Define TRANSACTION_SERVICE_URL holding the host information (production/staging/rinkeby/mainnet)

Describe the solution you'd like
We need to fetch and cache the full list of tokens so that we can appropriately classify Transfer transactions and attach the required data to the response for correct rendering in the UI.

The token info can be found in the /tokens/ endpoint of the Safe Transaction service. Be mindful that the result is paginated (maybe we should set a large limit?)

This endpoint returns ERC20 and ERC721, we should later on serialise the data polymorphically to prevent excessive optionals in the apps. (ERC20, ERC721 and Ether would be the cases thus far).

Additional context
This is required, as currently the iOS app pre-fetches and caches the full list of tokens whereas the Android app simply looks at the dataDecoded field of the transaction and sometimes misinterprets the data. This information should be included in the Transfer object for Multisig transaction mapping #27

• Build docker container
• Upload to dockerhub

References

• https://github.com/gnosis/safe-transaction-service/blob/master/scripts/deploy_docker.sh

• transaction service endpoint
• name
• version
• (Optional) service version
• (Optional) buildnumber or commit

Is your feature request related to a problem? Please describe.
Module transactions should be mapped (at least so far) only into Custom as defined in #24

Describe the solution you'd like
There should be a mechanism to detect and map any ModuleTransaction into a CustomTransaction

Additional context
For reference, the classes described in #24 will be used. For mapping logic the TransactionRepository implementation will be used and filling out of data will be done as needed.

Is your feature request related to a problem? Please describe.
Currently, the safe creation transaction is available as a separate endpoint in the transaction service.
For example: https://safe-transaction.gnosis.io/api/v1/safes/0xA063Cb7CFd8E57c30c788A0572CBbf2129ae56B6/creation/

In order to integrate it into the transaction list on the client, the client app needs then to make additional request when the last page of the all-transactions is reached, and then augment the response with the response from the endpoint above.

Describe the solution you'd like
The solution would be to receive the safe creation transaction from the transactions response. If the creation endpoint response structure is preserved, then we need to augment it at least with a new "txType" for the creation transaction. Alternatively, if there's a way to format the response as another 'settings' transaction, that could also work.

Describe alternatives you've considered
Alternative: to request the creation transaction data from the client

Additional context
This appeared as part of working on requirements for the issue 5afe/safe#315

Currently we only cache if an address is an token, but when an address is not an token we perform the request again.

Add tests to existing test.rs file. Extract json.

• ERC20 transfer summary
• ERC721 transfer summary
• Ether transfer summary
• Custom transaction summary
• Settings change summary

Currently the clients get push notifications that only contain the safeTxHash not the id generated by the client gateway. Therefore this should be possible to be used.

Describe the bug
TransferInfo structs serialise field names with lower case snake casing and the apps usually expect camel casing.

To Reproduce
call the transactions/{safe_address} endpoint and find an ERC20 transfer

Expected behavior
All field names should be camel cased and enums SCREAMING_SNAKE_CASED

Describe the solution you'd like
converters modules contain the core logic of the gateway and as such should be rigorously tested
This issue can be close when these issues are completed:

• #69
• #106

Describe alternatives you've considered
Test should be done against json taken from transaction service API

Additional context
There are tests also missing for the multisig case that should be addressed see #69

Is your feature request related to a problem? Please describe.
Ethereum transactions should be mapped (at least so far) only into Custom and Transfer transactions as defined in #24

Describe the solution you'd like
There should be a mechanism to detect and map any EthereumTransaction into a CustomTransaction and TransferTransaction.

The decision for each mapping is:

• Is the EthereumTransaction list of transfers not empty? If so expand the list to contain one domain Transfer from #24 for each TransferDto coming from the backend
• Is the EthereumTransaction list of transfers empty and data is not null or empty? then map into a CustomTransaction from #24
• else -> Map into a domainTransferTransaction using the parent EthereumTransaction struct as the base for mapping.

Additional context
For reference, the classes described in #24 will be used. For mapping logic the TransactionRepository implementation will be used and filling out of data will be done as needed.

• Proper status codes and error messages should be used (e.g. 400 for invalid ids instead of 500)

• Improve how network errors are handled internally (e.g. https://github.com/gnosis/safe-client-gateway/pull/103/files#diff-38f42f16fddf9ebfffe7f77c441e1fd5R62) so that appropriate messages can be generated

Is your feature request related to a problem? Please describe.
The endpoints naming are not compliant with the standard.

Describe the solution you'd like
Endpoints should be renamed. Proposal:
/transactions/<safe_address>?<next> -> /v1/safes/<safe_address>/transactions?<next>
/transactions/<tx_hash> -> /v1/transactions/<tx_hash>
/transactions/about -> /about

Additional context
https://restfulapi.net/resource-naming/

TBD

Is your feature request related to a problem? Please describe.
Currently paged data returns the links provided by the backend as the next and previous links for pagination. This will direct the clients to the service we are trying to map from.

Describe the solution you'd like
We want to map these URLs so that we can process them in this service.

Additional context
https://github.com/rmeissner/safe-merc-service/blob/master/src/renderer/overview.rs#L44

Describe the bug
Custom transaction is displayed instead of Change Mastercopy when the contract version is updated in gnosis safe web app through settings
The same for ios

To Reproduce
Steps to reproduce the behavior:

1. Load safe with old contact version ( change contract version from 1.1.1 to 1.0.0 - short instruction https://docs.google.com/spreadsheets/d/1J4bTGU3tute-DurP-v-YpV-3H59ptuIG0xSCC3Kl_1s/edit?usp=sharing )
2. Go to the settings in the gnosis safe and press Update
3. Go to the transactions in web app
4. Contract upgrade tx is created on UI
5. Check the same tx in the mobile app

Safe with wrong tx
0x1230B3d59858296A31053C1b8562Ecf89A2f888b
tx nonce 56

Expected behavior
Change Mastercopy transaction should be created if the mastercopy is upgraded to the new or downgraded

Smartphone (please complete the following information):

• Device: Pixel 4
• Android Version: 10
• App Version 0.4.2- alpha-debug

Additional context
Add any other context about the problem here.

Is your feature request related to a problem? Please describe.
The SafeInfo is required for calculating the status (according to #24 ). Particularly, the threshold and nonce are necessary to determine certain statuses.

Describe the solution you'd like
SafeInfo should be fetch from the /safe/{address} endpoint and cached.

Additional context
The class layout should look like in the swagger. Here is the Kotlin equivalent:

data class SafeInfo(
    val address: Solidity.Address,
    val nonce: BigInteger,
    val threshold: Int,
    val owners: List<Solidity.Address>,
    val masterCopy: Solidity.Address,
    val modules: List<Solidity.Address>,
    val fallbackHandler: Solidity.Address?
)

According to https://docs.google.com/document/d/1vhTu5NbB-6m8nE3cqTS3bRONigPO3NA5YkJqNTA6AOk/edit?skip_itp2_check=true a transfer that has the current safe as the tx initiator but neither to or from (row 10 in the table) , should be regarded as a custom transaction.

Here is an example:

{
      "safe": "0x938bae50a210b80EA233112800Cd5Bc2e7644300",
      "to": "0xc778417E063141139Fce010982780140Aa0cD5Ab",
      "value": "0",
      "data": "0x23b872dd0000000000000000000000001230b3d59858296a31053c1b8562ecf89a2f888b000000000000000000000000f2565317f3ae8ae9ea98e9fe1e7fadc77f823cbd00000000000000000000000000000000000000000000000000038d7ea4c68000",
      "operation": 0,
      "gasToken": "0x0000000000000000000000000000000000000000",
      "safeTxGas": 59217,
      "baseGas": 0,
      "gasPrice": "0",
      "refundReceiver": "0x0000000000000000000000000000000000000000",
      "nonce": 34,
      "executionDate": "2020-07-24T15:23:11Z",
      "submissionDate": "2020-07-24T15:23:11Z",
      "modified": "2020-07-24T15:23:11Z",
      "blockNumber": 6896576,
      "transactionHash": "0xc2ef20669265c7d448beb9f51d81c245dda735ae716888ab03e46b2eda16167f",
      "safeTxHash": "0x45228f489cdcf8b8fbf790ba959044ad69ab044d329b1dce0c0830329c7cfffb",
      "executor": "0xF2CeA96575d6b10f51d9aF3b10e3e4E5738aa6bd",
      "isExecuted": true,
      "isSuccessful": true,
      "ethGasPrice": "1000000000",
      "gasUsed": 79626,
      "fee": "79626000000000",
      "origin": null,
      "dataDecoded": {
        "method": "transferFrom",
        "parameters": [
          {
            "name": "from",
            "type": "address",
            "value": "0x1230B3d59858296A31053C1b8562Ecf89A2f888b"
          },
          {
            "name": "to",
            "type": "address",
            "value": "0xf2565317F3Ae8Ae9EA98E9Fe1e7FADC77F823cbD"
          },
          {
            "name": "value",
            "type": "uint256",
            "value": "1000000000000000"
          }
        ]
      },
      "confirmationsRequired": 1,
      "confirmations": [
        {
          "owner": "0xF2CeA96575d6b10f51d9aF3b10e3e4E5738aa6bd",
          "submissionDate": "2020-07-24T15:23:38.979651Z",
          "transactionHash": null,
          "confirmationType": "CONFIRMATION",
          "signature": "0x000000000000000000000000f2cea96575d6b10f51d9af3b10e3e4e5738aa6bd000000000000000000000000000000000000000000000000000000000000000001",
          "signatureType": "APPROVED_HASH"
        }
      ],
      "signatures": "0x000000000000000000000000f2cea96575d6b10f51d9af3b10e3e4e5738aa6bd000000000000000000000000000000000000000000000000000000000000000001",
      "transfers": [
        
      ],
      "txType": "MULTISIG_TRANSACTION"
    }

This can be found in a response to: https://safe-transaction.staging.gnosisdev.com/api/v1/safes/0x938bae50a210b80EA233112800Cd5Bc2e7644300/all-transactions/?limit=100

Look for "nonce":34

The mobile apps are supposed to treat this not as a transfer but as a Custom tx.
The gateway could emit this as a custom transfer.