Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction arguments parsing issue in Aptos transaction blind signing #3899

Open
wayne-law opened this issue Jun 15, 2024 · 10 comments
Open
Labels
bug Something isn't working

Comments

@wayne-law
Copy link

wayne-law commented Jun 15, 2024

Describe the bug
I'm trying to use a payloadJson to do an aptos transaction blind signing, but I found that the trust core will change the arguments's value in the encoded of output, it will cause the transaction go to fail.

here is the failed tx: https://aptoscan.com/transaction/983120591#payload

To Reproduce

  1. use this payload json to do an aptos transaction blind signing
{
    "type": "entry_function_payload",
    "function": "0x111ae3e5bc816a5e63c2da97d0aa3886519e0cd5e4b046659fa35796bd11542a::router::deposit_and_stake_entry",
    "type_arguments": [],
    "arguments": [
        "20000000",
        "0xb26df6e5f2a60248ab61deff98c6c45e0e8f16fdc5fc5e417e4e4d3b447aefc3"
    ]
}

here's an exmaple to do an aptos transaction blind signing: https://github.com/trustwallet/wallet-core/blob/master/android/app/src/androidTest/java/com/trustwallet/core/app/blockchains/aptos/TestAptosSigner.kt

  1. then use the encoded of output to broadcast
  2. go to aptoscan to check the transaction's payload, the arguments 0 changes to "0x002d310100000000"

Expected behavior
Don't change the value of argument in the transaction payload's arguments when do the signing

@wayne-law wayne-law added the bug Something isn't working label Jun 15, 2024
@jianbinking
Copy link

face the same issue with the same dapp, quite amazing 🤔
https://explorer.aptoslabs.com/txn/983156702/userTxnOverview?network=mainnet

@wayne-law
Copy link
Author

wayne-law commented Jun 15, 2024

Here's another failed tx: https://explorer.aptoslabs.com/txn/0x154cea3fe33ca021694fb1b8c0ae6ef80ab3e2df761a7041124fa48fbd239dc2/payload?network=mainnet

the raw payload is

{
    "type": "entry_function_payload",
    "function": "0x9770fa9c725cbd97eb50b2be5f7416efdfd1f1554beb0750d4dae4c64e860da3::controller::deposit",
    "type_arguments": [
        "0x1::aptos_coin::AptosCoin"
    ],
    "arguments": [
        "0x4d61696e204163636f756e74",
        "10000000",
        false
    ]
}

after signed by trust core, argument 0 change to "0x00000000000000000000000000000000000000004d61696e204163636f756e74".

Here's the success one signed by martian wallet extension:
https://explorer.aptoslabs.com/txn/0x894c2e80803996523a633bbe1383e737a10fdef050f07fa201b43b9d73c824de/payload?network=mainnet

@satoshiotomakan
Copy link
Collaborator

Hi @wayne-law, could you please advise how do you broadcast the signed transaction? We do recommend to broadcast SigningOutput.json value.
Regarding 0x00000000000000000000000000000000000000004d61696e204163636f756e74 value, it seems to be the same as 0x4d61696e204163636f756e74, just displayed with a prefixed zeros that should not make any sense

@10gic
Copy link
Contributor

10gic commented Oct 16, 2024

Hi @satoshiotomakan, could you please find someone to address this issue? The root cause is that deserializing the EntryFunction from JSON is ambiguous, as I mentioned in the pr #4011.

@satoshiotomakan
Copy link
Collaborator

Hi @10gic, I'll review the PR today, thank you for the reminder

@satoshiotomakan
Copy link
Collaborator

@10gic @wayne-law the issues are different.

  1. https://aptoscan.com/transaction/983120591#payload failed due to an incorrect Address argument type serialization. [Aptos]: Fix an issue in deserializing the EntryFunction object #4011 PR fixes the issue. I validated the fix via https://{APTOS_NODE}/v1/transactions/encode_submission
  2. However, the https://explorer.aptoslabs.com/txn/0x154cea3fe33ca021694fb1b8c0ae6ef80ab3e2df761a7041124fa48fbd239dc2/payload?network=mainnet transaction failed because we can't handle the 0x4d61696e204163636f756e74 value properly. It should be serialized as U8Vector, but we serializes it as Address with the prepended zeros (up to 32 bytes).
    We rely on the official move-aptos library. Looks like, Aptos nodes use a different implementation.
    For example,x\"deadbeef\" is a U8Vector representation, but not 0xdeadbeef: https://github.com/move-language/move-on-aptos/blob/f28f2429008535102f9b2fdbdaadc04507e2dce8/language/move-core/types/src/parser.rs#L470-L471

@satoshiotomakan
Copy link
Collaborator

Update: Aptos node is able to get expected calling function argument types, and based on expected types, it parses the given JSON argument values:
https://github.com/aptos-labs/aptos-core/blob/0e4f5df59f7518ddd82b9116a7cb942bfa658741/api/types/src/convert.rs#L874-L903

Unfortunately, we can't do the same in WalletCore as we don't have access to the network. However, we should probably consider taking the contract ABI as a parameter in SigningInput. @10gic @Milerius wdyt?

@10gic
Copy link
Contributor

10gic commented Oct 16, 2024

Hi @satoshiotomakan, I agree with you that we should pass the ABI into the wallet core and then use that information to reconstruct the EntryFunction.

@satoshiotomakan
Copy link
Collaborator

@10gic @wayne-law I scheduled a work on adding the ABI into SigningInput, but can't say when I'll be able to do it. Unfortunately
Hope coming iterations

@satoshiotomakan
Copy link
Collaborator

Please also note that an issue related to this failed transaction https://aptoscan.com/transaction/983120591#payload has been fixed by @10gic at #4011
Thanks @10gic 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants