PR: Fix inconsistent method decoding in data-decoder endpoint, Resolve #2508

[alfredolopez80]

PR: Fix inconsistent method decoding in data-decoder endpoint

Resolves #2508

Description

This PR addresses the issue where the /api/v1/data-decoder endpoint fails to decode methods from verified contracts that have their ABIs loaded in the transaction service.

The core problem was in the get_abi_function method of the DbTxDecoder class, which first checked if a function selector existed in the preloaded ABIs before attempting to use contract-specific ABIs. This meant that contract methods not included in the hardcoded ABIs (decoder_abis directory) or not loaded at service initialization time were never decoded, even if the contract was verified and its ABI was available in the database.

Before & After Examples

Mainnet Example

Contract: 0xB4EFd85c19999D84251304bDA99E90B92300Bd93
Method: setSaleContractFinalised

Method: setSaleContractFinalised (not in decoder_abis)

curl -X POST 'https://safe-transaction-mainnet.safe.global/api/v1/data-decoder/' \
-H 'accept: application/json' \
-H 'Content-Type: application/json' \
-d '{"data": "0x4a5c5e2e000000000000000000000000d8da6bf26964af9d7eed9e03e53415d37aa96045", "to": "0xB4EFd85c19999D84251304bDA99E90B92300Bd93"}'

Before:

{"method":"fallback","parameters":[]}

After:

{
  "method": "setSaleContractFinalised",
  "parameters": [
    {
      "name": "_sender",
      "type": "address",
      "value": "0xd8da6bf26964af9d7eed9e03e53415d37aa96045"
    }
  ]
}

Polygon Example

Contract: 0x062FfE63b7A0d7f27A8105e717c6Ea45E5848AD3
Method: add

Method: add (not in decoder_abis)

curl -X POST 'https://safe-transaction-polygon.safe.global/api/v1/data-decoder/' \
-H 'accept: application/json' \
-H 'Content-Type: application/json' \
-d '{"data": "0x0a97563f0000000000000000000000000000000000000000000000000000000000000064000000000000000000000000d8da6bf26964af9d7eed9e03e53415d37aa96045000000000000000000000000b4efd85c19999d84251304bda99e90b92300bd93", "to": "0x062FfE63b7A0d7f27A8105e717c6Ea45E5848AD3"}'

Before:

{"method":"fallback","parameters":[]}

After:

{
  "method": "add",
  "parameters": [
    {
      "name": "allocPoint",
      "type": "uint256",
      "value": "100"
    },
    {
      "name": "_lpToken",
      "type": "address",
      "value": "0xd8da6bf26964af9d7eed9e03e53415d37aa96045"
    },
    {
      "name": "_rewarder",
      "type": "address",
      "value": "0xb4efd85c19999d84251304bda99e90b92300bd93"
    }
  ]
}

Base Example

Contract: 0x2C8bC3198903DE6b61C1E1aA449578863Af3ccE7
Method: initialize

Method: initialize (not in decoder_abis)

curl -X POST 'https://safe-transaction-base.safe.global/api/v1/data-decoder/' \
-H 'accept: application/json' \
-H 'Content-Type: application/json' \
-d '{"data": "0xc4d66de8000000000000000000000000d8da6bf26964af9d7eed9e03e53415d37aa96045", "to": "0x2C8bC3198903DE6b61C1E1aA449578863Af3ccE7"}'

Before:

{"method":"fallback","parameters":[]}

After:

{
  "method": "initialize",
  "parameters": [
    {
      "name": "admin",
      "type": "address",
      "value": "0xd8da6bf26964af9d7eed9e03e53415d37aa96045"
    }
  ]
}

Solution

The solution inverts the order of ABI checking:

1. First, check contract-specific ABIs from the database when an address is provided
2. Only fall back to preloaded ABIs if no match is found in the contract-specific ABIs

def get_abi_function(
    self, data: bytes, address: Optional[ChecksumAddress] = None
) -> Optional[ABIFunction]:
    """
    :param data: transaction data
    :param address: contract address in case of ABI colliding
    :return: Abi function for data if it can be decoded, `None` if not found
    """
    selector = data[:4]
    
    # First try to use specific ABI if address is provided
    if address:
        contract_selectors_with_abis = self.get_contract_abi(address)
        if contract_selectors_with_abis and selector in contract_selectors_with_abis:
            return contract_selectors_with_abis[selector]
    
    # Then fall back to preloaded ABIs
    if selector in self.fn_selectors_with_abis:
        return self.fn_selectors_with_abis[selector]
    
    return None

Benefits

This change ensures:

1. Contract-specific ABIs are always prioritized when an address is provided
2. Methods not in the hardcoded ABIs can now be decoded if the contract is verified
3. The service can properly decode methods from contracts added to the database after service initialization

Testing

Two tests have been added to verify the fix:

1. test_db_tx_decoder_with_contract_specific_abi: Verifies that contract-specific ABIs are used even when the method selector isn't in the preloaded ABIs
2. test_db_tx_decoder_priority: Confirms that contract-specific ABIs take priority over preloaded ABIs when there's a conflict

These tests ensure that the data-decoder endpoint will now correctly handle all methods from verified contracts across all networks.

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[alfredolopez80]

I have read the CLA Document and I hereby sign the CLA

[Uxio0]

Hi @alfredolopez80 , we appreciate your PR a lot, we are only maintaining the decoder in the transaction service due to legacy compatibility, this is our new decoder service: https://github.com/safe-global/safe-decoder-service/

You can try it here: https://safe-decoder.safe.global/

[Uxio0]

My problem with your solution is that it will require always a call to the database, making returning transactions slower.

That's why we decided to make it a different service, so we can introduce more logic without affecting the tx service endpoints timing