EIP1271 offchain signature not merged before simulate/execute

jsvisa commented 3 months ago

Bug description

Environment

Browser: Chrome
Wallet: MetaMask
Chain: Ethereum sepolia

Steps to reproduce

I've depolyed a Safe 1.3.0 Wallet with 5 owners(threshold is 3), and one is another Safe Wallet, the others are EOA accounts.
Initialize a new transaction in safe' website
Confirm with another EOA account(now it has 2 EOA's approve)
1. Confirm with the other SafeWallet's approve with python script

from eth_account import Account

from gnosis.safe.safe import Safe
from gnosis.eth.ethereum_client import EthereumClient
from gnosis.safe.safe_signature import SafeSignatureContract
from gnosis.safe.api.transaction_service_api import TransactionServiceApi

eth_client = EthereumClient("https://sepolia.gateway.tenderly.co")

safe = Safe("0x5957cd3Aa492cf71E5EB957c7Fa40cE7C73b9737", eth_client)

con0 = Safe("0xbdA31fFC37e259468d55Afa509DEcF532EA29248", eth_client)
con0_eoa1_key = "contract safe eoa1's key"
con0_eoa2_key = "contract safe eoa2's key"

con0_eoa1 = Account.from_key(con0_eoa1_key)
con0_eoa2 = Account.from_key(con0_eoa2_key)

tx_service = TransactionServiceApi(eth_client.get_network(), eth_client)

safe_hash = "0x75b645b7c7fb5ce17443a9a130032576ccb051e2872aad52b3e1613e0030cd8e"
safe_tx, _ = tx_service.get_safe_transaction(safe_hash)
safe_tx_hash_preimage = safe_tx.safe_tx_hash_preimage
safe_tx_hash = safe_tx.safe_tx_hash

con0_message_hash = con0.get_message_hash(safe_tx_hash_preimage)
con0_eoa1_signature = con0_eoa1.signHash(con0_message_hash)["signature"]
con0_eoa2_signature = con0_eoa2.signHash(con0_message_hash)["signature"]
con0_compact_signature = (
    con0_eoa1_signature + con0_eoa2_signature
    if con0_eoa1.address.lower() < con0_eoa2.address.lower()
    else con0_eoa2_signature + con0_eoa1_signature
)

# Build EIP1271 signature v=0 r=safe v=dynamic_part dynamic_part=size+owner_signature
con0_contract = SafeSignatureContract.from_values(
    con0.address,
    safe_tx_hash,
    safe_tx_hash_preimage,
    con0_compact_signature,
)

tx_service.post_signatures(safe_tx_hash, con0_contract.export_signature())

Now it has 3 approves

Now I click the simulate button, but it failed:

Tenderly's link https://dashboard.tenderly.co/public/safe/safe-apps/simulator/ff6bbdae-1431-4a70-8a99-553ba01cd5cb

Seems the input is incorrect

0x6a76120200000000000000000000000050fd34b3a05499f547f720f9ae3142d9e06ee6fc0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000014000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001c0000000000000000000000000000000000000000000000000000000000000004440c10f1900000000000000000000000050fd34b3a05499f547f720f9ae3142d9e06ee6fc0000000000000000000000000000000000000000000000000000000000002b75000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000165000000000000000000000000bda31ffc37e259468d55afa509decf532ea292480000000000000000000000000000000000000000000000000000000000000041000000000000000000000000000000000000000000000000000000000000000082f7c05ca66f19e6999c7971c74b653de1d61da8f07d1bbeed1ab2bb0e5e7038e877354622f5f09e4c27662fb3bc008600ea4d8e834c88befe53191744d19439791ccb43709e014efc347ddcd2e23a98f724826bb58b7fa78d18ca1cdac90f16417b7c127251ac378bfed1b237871bb502fd7f602a26a77a6745351c8534f88336be1bbb3031480b229775a7262931cda1e988927d189144fd8366597f1f7293d9d63111f3fe6619ae8ae381ee2a45bce3ccbd6b79c90c4fa7f08841f13db94d90c7ec1cc0fd572f145074c8ae88b28bba4f1e59bdb3494d8cf08e1fe9855fa6a5303b1c39cf1ee704e0b055c690b97a43c742b6ad68006d5004c5995996491a4586fbab1b000000000000000000000000000000000000000000000000000000

We should put the contract's dynamic signature in the ending, but it is simple concated in the input

Expected result

Merge the signature before simulate or execute the transaction.

So I execute the transaction with the merged signature in python, it succeed:

from eth_account import Account
import requests
from hexbytes import HexBytes

from gnosis.eth.ethereum_client import EthereumClient
from gnosis.safe.safe_signature import SafeSignature
from gnosis.safe.api.transaction_service_api import TransactionServiceApi

eth_client = EthereumClient("https://sepolia.gateway.tenderly.co")
tx_service = TransactionServiceApi(eth_client.get_network(), eth_client)

safe_hash = "0x75b645b7c7fb5ce17443a9a130032576ccb051e2872aad52b3e1613e0030cd8e"
safe_tx, _ = tx_service.get_safe_transaction(safe_hash)
safe_tx_hash_preimage = safe_tx.safe_tx_hash_preimage
safe_tx_hash = safe_tx.safe_tx_hash

tx = requests.get(
    f"https://safe-transaction-sepolia.safe.global/api/v1/multisig-transactions/{safe_hash}/"
).json()

signatures = [
    SafeSignature.parse_signature(HexBytes(e["signature"]), safe_tx_hash)[0]
    for e in sorted(tx["confirmations"], key=lambda x: int(x["owner"], 16))
]

safe_tx.signatures = SafeSignature.export_signatures(signatures)

tx_call = safe_tx.call()
assert tx_call == 1, f"safe_tx call not equal: {tx_call}"

eoa = Account.from_key("eoa's private key")
print("SUCCESS -> ", safe_tx.execute(eoa.key))

jsvisa commented 3 months ago

Seems it's something like this one https://github.com/safe-global/safe-wallet-web/issues/1886#issuecomment-1537075441

The EIP1271 integration is merged in https://github.com/safe-global/safe-core-sdk/pull/556

But curious, on web's side, it did encode the signatures before execute the tx

https://github.com/safe-global/safe-core-sdk/blob/d5927980cae6d84d2a58fe3cf45931339fea0506/packages/protocol-kit/src/adapters/web3/contracts/Safe/SafeContractWeb3.ts#L168