[Multisig V2][API] Fix the API error for Multisig V2 #12445

junkil-park · 2024-03-08T23:37:20Z

Description

Currently, simulating Multisig payloads fails with the following error message:

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

This is caused by the incorrect serialization of the MultisigTransactionPayload type.
For the correct (de)serialization, this PR adds the serde attribute (tag = "type") to the enum type MultisigTransactionPayload

This PR resolves #12469,
and partially for #8304.

Test Plan

tested on the localnet

trunk-io · 2024-03-08T23:37:24Z

⏱️ 17h 23m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-unit-tests	3h 35m	🟥 ⬜ 🟥 🟩 🟩 (+3 more)
rust-smoke-tests	2h 37m	🟩 🟩 ⬜ 🟩 🟩 (+1 more)
execution-performance / single-node-performance	2h 13m	🟩 🟩 🟩 🟥 🟩 (+1 more)
windows-build	2h 6m	🟩 🟩 🟩 🟩 🟩 (+3 more)
rust-images / rust-all	1h 19m	🟩 🟩 ⬜ 🟩 🟩 (+1 more)
forge-e2e-test / forge	1h 10m	🟩 🟩 🟩 🟩 🟩
forge-compat-test / forge	1h 10m	🟩 🟩 🟩 🟩 🟩
rust-lints	53m	🟩 ⬜ 🟩 🟩 🟩 (+3 more)
cli-e2e-tests / run-cli-tests	33m	🟩 🟩 🟩 🟩 🟩
run-tests-main-branch	32m	🟩 🟥 🟥 🟥 🟥 (+3 more)
check	29m	🟩 ⬜ 🟩 🟩 🟩 (+3 more)
check-dynamic-deps	15m	🟩 🟩 🟩 🟩 🟩 (+3 more)
general-lints	15m	🟩 ⬜ 🟩 🟩 🟩 (+3 more)
node-api-compatibility-tests / node-api-compatibility-tests	4m	🟥 🟥 🟩 🟩 🟩
semgrep/ci	3m	🟩 🟩 🟩 🟩 🟩 (+3 more)
file_change_determinator	3m	🟩 🟩 🟩 🟩 🟩 (+3 more)
determine-docker-build-metadata	2m	🟩 🟩 🟩 🟩 🟩 (+1 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+3 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
execution-performance / file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	28s	🟩 🟩 🟩 🟩 🟩 (+3 more)
permission-check	28s	🟩 🟩 🟩 🟩 🟩 (+3 more)
permission-check	22s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	21s	🟩 🟩 🟩 🟩 🟩 (+3 more)
permission-check	17s	🟩 🟩 🟩 🟩 🟩 (+3 more)

🚨 1 job on the last run was significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
windows-build	12m	20m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

gregnazario · 2024-03-08T23:51:15Z

api/types/src/transaction.rs

 #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, Union)]
+#[serde(tag = "type", rename_all = "snake_case")]
+#[oai(one_of, discriminator_name = "type", rename_all = "snake_case")]
 pub enum MultisigTransactionPayload {
    EntryFunctionPayload(EntryFunctionPayload),
 }


Does this affect the indexer?

@bowenyang007 , @larry-aptos , we are making this change for the correct (de)serialization of simulation API results. Do you think this would affect the indexer?

I don't believe this affects indexer, we have our own layer to convert the API types into proto types and this doesn't leverage the JSON representation of the types from serde.

Thank you for the clarification!

junkil-park · 2024-03-09T01:53:18Z

api/types/src/transaction.rs

@@ -740,6 +740,8 @@ impl TryFrom<Script> for ScriptPayload {
 // We use an enum here for extensibility so we can add Script payload support
 // in the future for example.
 #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, Union)]
+#[serde(tag = "type", rename_all = "snake_case")]
+#[oai(one_of, discriminator_name = "type", rename_all = "snake_case")]


@banool , does this line for oai look good to you?

Yeah looks good.

junkil-park · 2024-03-09T01:57:47Z

@alnoki , this is the API error fix for the multisig payload simulation. I am currently investigating the remaining multisig simulation error.

alnoki · 2024-03-10T22:30:58Z

@alnoki , this is the API error fix for the multisig payload simulation. I am currently investigating the remaining multisig simulation error.

@junkil-park thank you for flagging. Yes this looks like a fix for multisig payload simulation!

Tagging #8304 here, which captures other similar issues

In particular this comment: #8304 (comment)

- For the correct (de)serialization, added the serde attribute (tag = "type") to the enum type `MultisigTransactionPayload`

github-actions · 2024-03-15T02:05:41Z

✅ Forge suite `compat` success on `aptos-node-v1.9.5` ==> `aabd57c7088e89e74bc016f1cbc6dd47f669394b`

Compatibility test results for aptos-node-v1.9.5 ==> aabd57c7088e89e74bc016f1cbc6dd47f669394b (PR)
1. Check liveness of validators at old version: aptos-node-v1.9.5
compatibility::simple-validator-upgrade::liveness-check : committed: 6017 txn/s, latency: 5229 ms, (p50: 4800 ms, p90: 9300 ms, p99: 14800 ms), latency samples: 228660
2. Upgrading first Validator to new version: aabd57c7088e89e74bc016f1cbc6dd47f669394b
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 684 txn/s, latency: 34298 ms, (p50: 36300 ms, p90: 53400 ms, p99: 56100 ms), latency samples: 56780
3. Upgrading rest of first batch to new version: aabd57c7088e89e74bc016f1cbc6dd47f669394b
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 186 txn/s, submitted: 552 txn/s, expired: 366 txn/s, latency: 31484 ms, (p50: 26000 ms, p90: 61400 ms, p99: 67300 ms), latency samples: 16208
4. upgrading second batch to new version: aabd57c7088e89e74bc016f1cbc6dd47f669394b
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 1946 txn/s, latency: 15069 ms, (p50: 16300 ms, p90: 19200 ms, p99: 20000 ms), latency samples: 89560
5. check swarm health
Compatibility test for aptos-node-v1.9.5 ==> aabd57c7088e89e74bc016f1cbc6dd47f669394b passed
Test Ok

github-actions · 2024-03-15T02:06:07Z

✅ Forge suite `realistic_env_max_load` success on `aabd57c7088e89e74bc016f1cbc6dd47f669394b`

two traffics test: inner traffic : committed: 7412 txn/s, latency: 5279 ms, (p50: 5100 ms, p90: 6000 ms, p99: 13300 ms), latency samples: 3209680
two traffics test : committed: 100 txn/s, latency: 1916 ms, (p50: 1800 ms, p90: 2200 ms, p99: 5900 ms), latency samples: 1680
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.239, avg: 0.204", "QsPosToProposal: max: 0.399, avg: 0.297", "ConsensusProposalToOrdered: max: 0.485, avg: 0.454", "ConsensusOrderedToCommit: max: 0.330, avg: 0.311", "ConsensusProposalToCommit: max: 0.815, avg: 0.764"]
Max round gap was 2 [limit 4] at version 1489191. Max no progress secs was 7.806725 [limit 15] at version 1489191.
Test Ok

junkil-park requested review from banool, gregnazario and 0xmaayan as code owners March 8, 2024 23:37

junkil-park requested review from hardsetting, movekevin and gedigi March 8, 2024 23:37

gregnazario reviewed Mar 8, 2024

View reviewed changes

gregnazario approved these changes Mar 8, 2024

View reviewed changes

junkil-park commented Mar 9, 2024

View reviewed changes

movekevin approved these changes Mar 11, 2024

View reviewed changes

junkil-park enabled auto-merge (squash) March 12, 2024 00:13