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

[Feature Request] Add support for simulating already created multisig transactions #8304

Open
alnoki opened this issue May 20, 2023 · 6 comments
Assignees
Labels
API bug Something isn't working move-framework Issues related to the Framework modules/libraries OPSQ323 ops week for q3 2023 stale-exempt Prevents issues from being automatically marked and closed as stale

Comments

@alnoki
Copy link
Contributor

alnoki commented May 20, 2023

@movekevin @gregnazario @davidiw

Summarized findings

This issue contains substantial documentation of error testing

See this summary comment at the bottom for a simple breakdown.

Background

This bug was uncovered while working on a PR to extend multisig support for the aptos CLI.

Once a multisig transaction was approved on-chain with enough votes, it could only be executed by specifying a --max-gas argument to the CLI execute subcommand, as this approach avoids calling the transaction simulator during this inner call.

Error messages

The relevant error thrown as a result of the simulator is:

{
  "Error": "API error: Unknown error error decoding response body: invalid value: map, expected map with a single key at line 1 column 855"
}

After tracing through the stack, it appears there is some kind of request response deserialization error here

Successful transaction

The successful transaction is question is here, for this multisig account, run with this command on the CLI:

aptos multisig execute \
    --multisig-address $multisig_addr \
    --function-id 0x1::coin::transfer \
    --type-args 0x1::aptos_coin::AptosCoin \
    --args \
        address:"$bee_addr" \
        u64:321 \
    --private-key-file bee.key \
    --max-gas 100000

Note the --max-gas argument for the modified CLI execute subcommand.

@alnoki alnoki added the bug Something isn't working label May 20, 2023
@alnoki
Copy link
Contributor Author

alnoki commented May 21, 2023

Additional related error

Note that this transaction (a package publication from a multisig account) executed successfully, but the CLI returned:

{
  "Error": "API error: Unknown error error decoding response body: invalid value: map, expected map with a single key at line 1 column 3730"
}

Here, the command that prompted the error was:

aptos multisig execute \
    --multisig-address $multisig_addr \
    --json-file publication.json \
    --private-key-file bee.key \
    --max-gas 10000

@alnoki alnoki changed the title [Bug] Transaction simulator is broken for multisig transactions [Bug] Transaction simulator and API response headers are broken for multisig transactions May 21, 2023
@alnoki
Copy link
Contributor Author

alnoki commented May 21, 2023

Different types of transactions

Upon further investigation it was revealed that different kinds of multisig transaction executions leads to different errors.

Simple entry function invocation

The transaction simulator appears to still be broken for multisig transaction execution via simple entry functions, but response headers are fine. For example consider the following transaction creation:

aptos multisig create-transaction \                                           
    --multisig-address $multisig_addr \
    --function-id $multisig_addr::cli_args::set_vals \
    --type-args \
        0x1::account::Account \
        0x1::chain_id::ChainId \
    --args \
        u8:123 \
        "bool:[false, true, false, false]" \
        'address:[["0xace", "0xbee"], ["0xcad"], []]' \
    --private-key-file bee.key

Once approved, a transaction execution attempt was invoked via:

aptos multisig execute \
    --multisig-address $multisig_addr \
    --private-key-file ace.key

This yielded the following simulation error:

{
  "Error": "Simulation failed with status: Execution failed in script at code offset 0"
}

When a --max-gas argument was specified, however, the function ran without issue and a transaction result was provided.

aptos multisig execute \
    --multisig-address $multisig_addr \
    --private-key-file ace.key \
    --max-gas 10000
Do you want to submit transaction for a maximum of 1000000 Octas at a gas unit price of 100 Octas? [yes/no] >
y
{
  "Result": {
    "transaction_hash": "0xb8513449bc75d3d9df99456b55e3a0aacf01b0c85e9e6ee55392c062e6188053",
    "gas_used": 3,
    "gas_unit_price": 100,
    "sender": "acee3447860cd5f14801badcbf69dbdb98a0c315999ded339bb9d3606ac4faa4",
    "sequence_number": 5,
    "success": true,
    "timestamp_us": 1684676634871902,
    "version": 525537197,
    "vm_status": "Executed successfully"
  }
}

The relevant transaction is here.

Code publication invocation

Per #7709, the PR branch adds a --json-output-file flag to aptos move publish, such that a package can be compiled into a public entry function JSON file in the style of #8054:

aptos move publish \
    --named-addresses test_account=$multisig_addr \
    --json-output-file publication.json

The resultant file looks like:

{
  "function_id": "0x1::code::publish_package_txn",
  "type_args": [],
  "args": [
    {
      "arg_type": "hex",
      "arg_value": "0x07436c6941726773010000000000000000403..."
    },
    {
      "arg_type": "hex",
      "arg_value": [
        "0xa11ceb0b060000000b010006020608030e..."
      ]
    }
  ]
}

This kind of file can be used to successfully publish a package from both a single-signer and a multisig account, as it enables a multisig owner to propose package publication from a JSON file and also to check the on-chain payload/hash. Transaction creation:

aptos multisig create-transaction \
    --multisig-address $multisig_addr \
    --json-file publication.json \
    --hash-only \
    --private-key-file ace.key
Do you want to submit a transaction for a range of [51000 - 76500] Octas at a gas unit price of 100 Octas? [yes/no] >
y
{
  "Result": {
    "transaction_hash": "0xfd48145327e10d5491f9e527aac362614e7b503bb4623c1260ed0b633d0fb499",
    "gas_used": 510,
    "gas_unit_price": 100,
    "sender": "acee3447860cd5f14801badcbf69dbdb98a0c315999ded339bb9d3606ac4faa4",
    "sequence_number": 6,
    "success": true,
    "timestamp_us": 1684677806015810,
    "version": 525545673,
    "vm_status": "Executed successfully"
  }
}

Transaction payload/hash checking:

aptos multisig check-transaction \
    --multisig-address $multisig_addr \
    --json-file publication.json \
    --sequence-number 4
{
  "Result": {
    "Status": "Transaction match",
    "Multisig transaction": {
      "creation_time_secs": "1684677806",
      "creator": "0xacee3447860cd5f14801badcbf69dbdb98a0c315999ded339bb9d3606ac4faa4",
      "payload": {
        "vec": []
      },
      "payload_hash": {
        "vec": [
          "0xce31dac5c29fd54c643119b4011a4991bd96141a21be10100d75336230417e89"
        ]
      },
      "votes": {
        "data": [
          {
            "key": "0xacee3447860cd5f14801badcbf69dbdb98a0c315999ded339bb9d3606ac4faa4",
            "value": true
          }
        ]
      }
    }
  }
}

Once enough approvals have been made, however, the transaction simulator fails during an execution attempt if no --max-gas argument is specified:

aptos multisig execute \
    --multisig-address $multisig_addr \
    --json-file publication.json \
    --private-key-file bee.key
{
  "Error": "API error: Unknown error error decoding response body: invalid value: map, expected map with a single key at line 1 column 10194"
}

If a --max-gas argument is specified the transaction will execute, but it can only be successfully verified on the blockchain explorer because there is an additional API response error that prevents inspection via the CLI:

aptos multisig execute \
    --multisig-address $multisig_addr \
    --json-file publication.json \
    --private-key-file bee.key \
    --max-gas 10000
Do you want to submit transaction for a maximum of 1000000 Octas at a gas unit price of 100 Octas? [yes/no] >
y
{
  "Error": "API error: Unknown error error decoding response body: invalid value: map, expected map with a single key at line 1 column 3730"
}

Note that the transaction in question was, however, successfully executed as described here

@alnoki
Copy link
Contributor Author

alnoki commented May 21, 2023

Hash vs payload behavior

Upon further investigation, it was determined that failure modes are a result of payload/hash status:

Stored on-chain Simulator works? Response decoding works?
Multisig payload No Yes
Multisig payload hash No No

In other words, if a multisig transaction is created with a full payload stored on-chain then trying to execute the multisig transaction without specifying --max-gas will result in an error of the form:

{
  "Error": "Simulation failed with status: Execution failed in script at code offset 0"
}

The transaction can still be executed from the command line, however, and will print a successful transaction result if the payload is well-formed.

In contrast, if a multisig transaction is created with only a payload hash stored on-chain then trying to execute the multisig transaction without specifying --max-gas will result in an error of the form:

{
  "Error": "API error: Unknown error error decoding response body: invalid value: map, expected map with a single key at line 1 column 10194"
}

In this case, even though the transaction can still be executed from the command line if the payload and hash are well-formed (using --max-gas to avoid invoking the transaction simulator), the transaction will have to be verified on the blockchain explorer because once the transaction has been executed the CLI errors out during the response decoding process:

{
  "Error": "API error: Unknown error error decoding response body: invalid value: map, expected map with a single key at line 1 column 3730"
}

@alnoki alnoki changed the title [Bug] Transaction simulator and API response headers are broken for multisig transactions [Bug][API] Txn simulator and response decoding is broken for multisig txn execution May 21, 2023
@alnoki alnoki changed the title [Bug][API] Txn simulator and response decoding is broken for multisig txn execution [Bug][API] Multisig txn execution breaks simulator, response decoder May 21, 2023
@movekevin
Copy link
Contributor

Thanks for reporting this and all the amazing details provided!

There are 2 types of simulation: of a non-existent multisig tx and of an existing one (explicit committed tx to create it). Currently we only support the former so you have to provide the transaction payload when running the simulation.

We'll need to add support for the latter - simulating execution of an existing multisig tx.

@movekevin movekevin added move-framework Issues related to the Framework modules/libraries API labels May 24, 2023
@movekevin movekevin moved this from 🆕 New to For Grabs in Move Language and Runtime May 28, 2023
@movekevin movekevin changed the title [Bug][API] Multisig txn execution breaks simulator, response decoder [Feature Request] Add support for simulating already created multisig transactions May 28, 2023
@gregnazario gregnazario moved this from 🎉 New to ⏳ Backlog in Developer Experience Jul 12, 2023
@github-actions
Copy link
Contributor

This issue is stale because it has been open 45 days with no activity. Remove the stale label or comment - otherwise this will be closed in 15 days.

@gregnazario
Copy link
Contributor

This should finally be supported in TypeScript. I think the CLI does this as well @junkil-park ?

@gregnazario gregnazario moved this from ⏳ Backlog to 🏗 In progress in Developer Experience Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API bug Something isn't working move-framework Issues related to the Framework modules/libraries OPSQ323 ops week for q3 2023 stale-exempt Prevents issues from being automatically marked and closed as stale
Projects
Status: 🏗 In progress
Development

No branches or pull requests

7 participants