[consensus observer] add new BlockTransactionPayload variants to prepare for block gas limit override on blocks #15848

bchocho · 2025-01-30T19:01:53Z

Description

These are the consensus observer message-side changes needed to support blocks having a block gas limit override. Once these messages are deployed, they can be used.

How Has This Been Tested?

Existing unit tests.

Key Areas to Review

Serde compatibility. Any structs that are currently used in messages should not be changed in place.

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

…are for block gas limit override on blocks

trunk-io · 2025-01-30T19:01:57Z

⏱️ 37m total CI duration on this PR

Slowest 15 Jobs	Cumulative Duration	Recent Runs
check-dynamic-deps	7m	🟩 🟩 🟩 🟩
rust-cargo-deny	7m	🟩 🟩 🟩 🟩
rust-doc-tests	6m	🟩
execution-performance / test-target-determinator	5m	🟩
test-target-determinator	5m	🟩
general-lints	2m	🟩 🟩 🟩 🟩
fetch-last-released-docker-image-tag	2m	🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩
file_change_determinator	42s	🟩 🟩 🟩 🟩
file_change_determinator	15s	🟩
execution-performance / single-node-performance	9s	🟩
permission-check	9s	🟩 🟩 🟩 🟩
permission-check	7s	🟩 🟩 🟩 🟩
permission-check	2s	🟩
determine-docker-build-metadata	1s	🟩

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

Job	Duration	vs 7d avg	Delta
execution-performance / single-node-performance	9s	20m

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

ibalajiarun · 2025-01-30T22:31:28Z

consensus/src/consensus_observer/network/observer_message.rs

+    transaction_limit: Option<u64>,
+    gas_limit: Option<u64>,


should we make this an enum, so we just need to extend this in future if necessary?

I was thinking this, but thought we probably don't need to extend it. Probably what we thought before too :P

@ibalajiarun changed to an enum. There's a lot of "similar-looking and sounding" structs in here now, that we'll need to clean up later

ibalajiarun · 2025-01-30T22:31:59Z

consensus/src/consensus_observer/network/observer_message.rs

+    OptQuorumStoreV2(
+        PayloadWithProofAndLimits,
+        /* OptQS and Inline Batches */ Vec<BatchInfo>,
+    ),


On second thought, we don't need a V2 for opt quorum store because we haven't turned it on yet?

JoshLind · 2025-01-31T19:33:39Z

consensus/src/consensus_observer/network/observer_message.rs

    QuorumStoreInlineHybrid(PayloadWithProofAndLimit, Vec<BatchInfo>),
    OptQuorumStore(
-        PayloadWithProofAndLimit,
+        TransactionsWithProof,


We're okay breaking the existing serialization for the OptQuorumStore?

^ yeah, this is from @ibalajiarun

On second thought, we don't need a V2 for opt quorum store because we haven't turned it on yet?

Awesome, thanks!

JoshLind

LGTM! Thanks @bchocho 😄

github-actions · 2025-01-31T20:09:51Z

✅ Forge suite `realistic_env_max_load` success on `7627a20eab3838fbf12edaf5f4362ff622564057`

two traffics test: inner traffic : committed: 14530.42 txn/s, latency: 2728.47 ms, (p50: 2400 ms, p70: 2700, p90: 3000 ms, p99: 8400 ms), latency samples: 5524940
two traffics test : committed: 99.96 txn/s, latency: 2399.82 ms, (p50: 1400 ms, p70: 1500, p90: 4000 ms, p99: 21400 ms), latency samples: 1720
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.267, avg: 1.141", "ConsensusProposalToOrdered: max: 0.301, avg: 0.295", "ConsensusOrderedToCommit: max: 0.438, avg: 0.420", "ConsensusProposalToCommit: max: 0.730, avg: 0.715"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.51s no progress at version 285 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.69s no progress at version 2756083 (avg 0.69s) [limit 16].
Test Ok

github-actions · 2025-01-31T20:10:01Z

✅ Forge suite `compat` success on `60f7ca8827f5d64a148c3b163dc4126b0879279b` ==> `7627a20eab3838fbf12edaf5f4362ff622564057`

Compatibility test results for 60f7ca8827f5d64a148c3b163dc4126b0879279b ==> 7627a20eab3838fbf12edaf5f4362ff622564057 (PR)
1. Check liveness of validators at old version: 60f7ca8827f5d64a148c3b163dc4126b0879279b
compatibility::simple-validator-upgrade::liveness-check : committed: 12641.97 txn/s, latency: 2461.82 ms, (p50: 2500 ms, p70: 2600, p90: 2800 ms, p99: 3100 ms), latency samples: 417740
2. Upgrading first Validator to new version: 7627a20eab3838fbf12edaf5f4362ff622564057
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 4366.77 txn/s, latency: 7002.04 ms, (p50: 7800 ms, p70: 8300, p90: 8700 ms, p99: 8700 ms), latency samples: 92960
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 4242.32 txn/s, latency: 7998.25 ms, (p50: 8800 ms, p70: 8900, p90: 9600 ms, p99: 9700 ms), latency samples: 151140
3. Upgrading rest of first batch to new version: 7627a20eab3838fbf12edaf5f4362ff622564057
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 4390.81 txn/s, latency: 7096.40 ms, (p50: 7800 ms, p70: 8400, p90: 8600 ms, p99: 8700 ms), latency samples: 91960
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 4407.12 txn/s, latency: 7700.04 ms, (p50: 8700 ms, p70: 8700, p90: 8700 ms, p99: 9100 ms), latency samples: 155800
4. upgrading second batch to new version: 7627a20eab3838fbf12edaf5f4362ff622564057
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 8275.50 txn/s, latency: 3725.53 ms, (p50: 4300 ms, p70: 4600, p90: 4700 ms, p99: 4800 ms), latency samples: 151200
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8089.08 txn/s, latency: 4237.53 ms, (p50: 4600 ms, p70: 4700, p90: 4900 ms, p99: 5000 ms), latency samples: 271540
5. check swarm health
Compatibility test for 60f7ca8827f5d64a148c3b163dc4126b0879279b ==> 7627a20eab3838fbf12edaf5f4362ff622564057 passed
Test Ok

github-actions · 2025-01-31T20:11:36Z

✅ Forge suite `framework_upgrade` success on `60f7ca8827f5d64a148c3b163dc4126b0879279b` ==> `7627a20eab3838fbf12edaf5f4362ff622564057`

Compatibility test results for 60f7ca8827f5d64a148c3b163dc4126b0879279b ==> 7627a20eab3838fbf12edaf5f4362ff622564057 (PR)
Upgrade the nodes to version: 7627a20eab3838fbf12edaf5f4362ff622564057
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1012.44 txn/s, submitted: 1017.34 txn/s, failed submission: 4.90 txn/s, expired: 4.90 txn/s, latency: 2786.06 ms, (p50: 1600 ms, p70: 2100, p90: 4500 ms, p99: 15100 ms), latency samples: 91001
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1022.21 txn/s, submitted: 1026.91 txn/s, failed submission: 4.70 txn/s, expired: 4.70 txn/s, latency: 2892.27 ms, (p50: 1800 ms, p70: 2400, p90: 11400 ms, p99: 13300 ms), latency samples: 91401
5. check swarm health
Compatibility test for 60f7ca8827f5d64a148c3b163dc4126b0879279b ==> 7627a20eab3838fbf12edaf5f4362ff622564057 passed
Upgrade the remaining nodes to version: 7627a20eab3838fbf12edaf5f4362ff622564057
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1722.76 txn/s, submitted: 1728.76 txn/s, failed submission: 6.00 txn/s, expired: 6.00 txn/s, latency: 1654.57 ms, (p50: 1500 ms, p70: 1800, p90: 2600 ms, p99: 3800 ms), latency samples: 154920
Test Ok

[consensus observer] add new BlockTransactionPayload variants to prep…

8aa965f

…are for block gas limit override on blocks

bchocho marked this pull request as ready for review January 30, 2025 22:14

bchocho requested review from zekun000, sasha8 and ibalajiarun as code owners January 30, 2025 22:14

bchocho requested a review from igor-aptos January 30, 2025 22:14

ibalajiarun reviewed Jan 30, 2025

View reviewed changes

bchocho added 2 commits January 30, 2025 14:48

remove unnecessary OptQuorumStoreV2

350aecf

Move the new BlockTransactionPayload to use enum TransactionsWithProof

7627a20

ibalajiarun approved these changes Jan 31, 2025

View reviewed changes

bchocho requested review from hariria and JoshLind January 31, 2025 19:23

JoshLind reviewed Jan 31, 2025

View reviewed changes

JoshLind approved these changes Jan 31, 2025

View reviewed changes

bchocho enabled auto-merge (squash) January 31, 2025 19:41

This comment has been minimized.

Sign in to view

bchocho merged commit fc4ae47 into main Jan 31, 2025
88 checks passed

bchocho deleted the brian/gas-limit-observer-messages-2 branch January 31, 2025 20:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[consensus observer] add new BlockTransactionPayload variants to prepare for block gas limit override on blocks #15848

[consensus observer] add new BlockTransactionPayload variants to prepare for block gas limit override on blocks #15848

bchocho commented Jan 30, 2025 •

edited

Loading

trunk-io bot commented Jan 30, 2025 •

edited

Loading

ibalajiarun Jan 30, 2025

bchocho Jan 30, 2025

bchocho Jan 30, 2025

ibalajiarun Jan 30, 2025

JoshLind Jan 31, 2025

bchocho Jan 31, 2025

JoshLind Jan 31, 2025

JoshLind left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 31, 2025

github-actions bot commented Jan 31, 2025

github-actions bot commented Jan 31, 2025

[consensus observer] add new BlockTransactionPayload variants to prepare for block gas limit override on blocks #15848

[consensus observer] add new BlockTransactionPayload variants to prepare for block gas limit override on blocks #15848

Conversation

bchocho commented Jan 30, 2025 • edited Loading

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

trunk-io bot commented Jan 30, 2025 • edited Loading

ibalajiarun Jan 30, 2025

Choose a reason for hiding this comment

bchocho Jan 30, 2025

Choose a reason for hiding this comment

bchocho Jan 30, 2025

Choose a reason for hiding this comment

ibalajiarun Jan 30, 2025

Choose a reason for hiding this comment

JoshLind Jan 31, 2025

Choose a reason for hiding this comment

bchocho Jan 31, 2025

Choose a reason for hiding this comment

JoshLind Jan 31, 2025

Choose a reason for hiding this comment

JoshLind left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 31, 2025

✅ Forge suite realistic_env_max_load success on 7627a20eab3838fbf12edaf5f4362ff622564057

github-actions bot commented Jan 31, 2025

✅ Forge suite compat success on 60f7ca8827f5d64a148c3b163dc4126b0879279b ==> 7627a20eab3838fbf12edaf5f4362ff622564057

github-actions bot commented Jan 31, 2025

✅ Forge suite framework_upgrade success on 60f7ca8827f5d64a148c3b163dc4126b0879279b ==> 7627a20eab3838fbf12edaf5f4362ff622564057

bchocho commented Jan 30, 2025 •

edited

Loading

trunk-io bot commented Jan 30, 2025 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `7627a20eab3838fbf12edaf5f4362ff622564057`

✅ Forge suite `compat` success on `60f7ca8827f5d64a148c3b163dc4126b0879279b` ==> `7627a20eab3838fbf12edaf5f4362ff622564057`

✅ Forge suite `framework_upgrade` success on `60f7ca8827f5d64a148c3b163dc4126b0879279b` ==> `7627a20eab3838fbf12edaf5f4362ff622564057`