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

validator txn api fix #13718

Merged
merged 9 commits into from
Jun 23, 2024
Merged

validator txn api fix #13718

merged 9 commits into from
Jun 23, 2024

Conversation

zjma
Copy link
Contributor

@zjma zjma commented Jun 15, 2024

Description

TLDR

  • For internal txn type ValidatorTransaction::JWKUpdate.
  • New JSON property validator_transaction_type, value being "observed_jwk_update".
  • New JSON property quorum_certified_update: details of the JWK updates.
  • For internal txn type ValidatorTransaction::DKGResult.
    • New JSON property validator_transaction_type, value being "dkg_result".
    • New JSON property dkg_transcript: details of the DKG transcript.
  • The following events are moved from BlockMetadataTransaction to ValidatorTransaction::DKGResult when randomness is enabled.
    • 0x1::stake::DistributeRewardsEvent
    • 0x1::stake::DistributeRewards
    • 0x1::reconfiguration::NewEpoch
    • 0x1::reconfiguration::NewEpochEvent

Example JSONs

example ValidatorTransaction::JWKUpdate json:

{
    "version": "2",
    "hash": "0xc52861c077546788b0cdd571935ef70de331b78411313e273f1a4c6110797342",
    "state_change_hash": "0xc5ad04ecdae5603876c077a635650195d2a166309894094b68e9630847a6da31",
    "event_root_hash": "0xd256b1260508902acf6ea7592fe8da54a76d9759a9f34569d77273302637329d",
    "state_checkpoint_hash": null,
    "gas_used": "0",
    "success": true,
    "vm_status": "Executed successfully",
    "accumulator_root_hash": "0xc630139580da3da1b896e690b9d4b558d77ac4950eb0a1e0236e1b9cc2a1abd6",
    "changes": [], // detail omitted...
    "events": [], //detail omitted...
    "timestamp": "1718424440685360",
    "validator_transaction_type": "observed_jwk_update", // new property!
    "type": "validator_transaction",
    "quorum_certified_update": {} // detail omitted...
}

example ValidatorTransaction::DKGResult json:

{
    "version": "9",
    "hash": "0x0bff715ffcf24ec4c0fb504da45059104774d52a1bcdc5eef1e574fcc6dd6840",
    "state_change_hash": "0xc74bb84ae5f647c60022d48de9e6bed5bb2e8dac6c8cd9b6ca5e26b3edf1cfbd",
    "event_root_hash": "0xe4e0955b86dad038a8dd96fe1ed67b1c893500d2e825e7fb41de4c0fa2325ae5",
    "state_checkpoint_hash": "0xef943d37d839f33b0416366b453ec682ef258ce1a08c5195e711277b7a32bbcd",
    "gas_used": "0",
    "success": true,
    "vm_status": "Executed successfully",
    "accumulator_root_hash": "0x4e579a1e2434bde316d597e8f2dd621371a95524891b0b7b44f69671d19bfef8",
    "changes": [], //detail omitted...
    "events": [ // these are in BlockMetadataTransaction when randomness is disabled!
        {
            "guid": {
                "creation_number": "0",
                "account_address": "0x0"
            },
            "sequence_number": "0",
            "type": "0x1::stake::DistributeRewards",
            "data": {
                "pool_address": "0xdf8211644b8da986605fd92f43d3fc823f49fff21891e78871ccfdb0d1608f47",
                "rewards_amount": "0"
            }
        },
        {
            "guid": {
                "creation_number": "12",
                "account_address": "0xdf8211644b8da986605fd92f43d3fc823f49fff21891e78871ccfdb0d1608f47"
            },
            "sequence_number": "0",
            "type": "0x1::stake::DistributeRewardsEvent",
            "data": {
                "pool_address": "0xdf8211644b8da986605fd92f43d3fc823f49fff21891e78871ccfdb0d1608f47",
                "rewards_amount": "0"
            }
        },
        {
            "guid": {
                "creation_number": "0",
                "account_address": "0x0"
            },
            "sequence_number": "0",
            "type": "0x1::reconfiguration::NewEpoch",
            "data": {
                "epoch": "2"
            }
        },
        {
            "guid": {
                "creation_number": "2",
                "account_address": "0x1"
            },
            "sequence_number": "1",
            "type": "0x1::reconfiguration::NewEpochEvent",
            "data": {
                "epoch": "2"
            }
        }
    ],
    "timestamp": "1718424443766159",
    "validator_transaction_type": "dkg_result", // new property!
    "type": "validator_transaction",
    "dkg_transcript": {}, // detail omitted...
}

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Other (specify)

How Has This Been Tested?

Key Areas to Review

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

Copy link

trunk-io bot commented Jun 15, 2024

⏱️ 10h 47m total CI duration on this PR
Job Cumulative Duration Recent Runs
test-fuzzers 2h 26m 🟩🟩🟩🟩
rust-targeted-unit-tests 2h 12m 🟩🟩🟩🟩 (+3 more)
rust-move-tests 1h 32m 🟩🟩🟩🟩 (+3 more)
rust-lints 46m 🟩🟩🟩🟩 (+3 more)
rust-smoke-tests 40m 🟩
run-tests-main-branch 37m 🟩🟩🟩🟩🟩 (+3 more)
execution-performance / single-node-performance 26m 🟥
rust-images / rust-all 16m 🟩
forge-framework-upgrade-test / forge 15m 🟩
general-lints 15m 🟩🟩🟩🟩🟩 (+3 more)
forge-e2e-test / forge 14m 🟩
forge-compat-test / forge 12m 🟩
execution-performance / test-target-determinator 12m 🟩
test-target-determinator 11m 🟩
check-dynamic-deps 10m 🟩🟩🟩🟩🟩 (+3 more)
cli-e2e-tests / run-cli-tests 7m 🟩
rust-build-cached-packages 5m 🟩
check 4m 🟩
semgrep/ci 3m 🟩🟩🟩🟩🟩 (+3 more)
file_change_determinator 1m 🟩🟩🟩🟩🟩 (+3 more)
file_change_determinator 1m 🟩🟩🟩🟩🟩 (+3 more)
node-api-compatibility-tests / node-api-compatibility-tests 56s 🟥
permission-check 23s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 21s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 20s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 19s 🟩🟩🟩🟩🟩 (+3 more)
file_change_determinator 10s 🟩
permission-check 2s 🟩
determine-docker-build-metadata 2s 🟩
🚨 6 jobs on the last run were significantly faster/slower than expected
Job Duration vs 7d avg Delta
execution-performance / test-target-determinator 12m 4m +167%
test-target-determinator 11m 4m +156%
execution-performance / single-node-performance 26m 11m +148%
rust-targeted-unit-tests 27m 13m +110%
rust-move-tests 16m 9m +81%
rust-lints 8m 6m +29%

settingsfeedbackdocs ⋅ learn more about trunk.io

@zjma zjma marked this pull request as ready for review June 15, 2024 07:32
api/types/src/transaction.rs Outdated Show resolved Hide resolved
api/types/src/transaction.rs Outdated Show resolved Hide resolved
types/src/block_metadata_ext.rs Outdated Show resolved Hide resolved
api/types/src/transaction.rs Outdated Show resolved Hide resolved
api/types/src/transaction.rs Outdated Show resolved Hide resolved
api/types/src/convert.rs Outdated Show resolved Hide resolved
types/src/block_metadata_ext.rs Outdated Show resolved Hide resolved
api/types/src/transaction.rs Outdated Show resolved Hide resolved
api/types/src/transaction.rs Outdated Show resolved Hide resolved
api/types/src/transaction.rs Outdated Show resolved Hide resolved
api/types/src/transaction.rs Outdated Show resolved Hide resolved
@zjma zjma changed the title api fix validator txn api fix Jun 18, 2024
@zjma zjma requested a review from gregnazario June 18, 2024 20:40
@zjma zjma enabled auto-merge (squash) June 21, 2024 23:18

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@larry-aptos
Copy link
Contributor

larry-aptos commented Jun 22, 2024

  1. update the transaction example that a new field is added to API dkg_transcript when the validator transaction type is dkg_result. It isn't very clear to understand the example..

  2. the changes seem to be in the API response but in the description, it's said omitted. You may want to update the example accordingly. The events look good.

Example:

image

@larry-aptos
Copy link
Contributor

We can index the quorum_certified_update or dkg_transcript fields when we have a better understanding of how these fields can be used in ecosystem projects.

i.e. we will surface them up in protobufs but not in actual indexing.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 25421b3195d4d779277f4f23e6fab0a357e98719

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 25421b3195d4d779277f4f23e6fab0a357e98719 (PR)
1. Check liveness of validators at old version: f648076a280621dbfd4e73b1ca83e3a3f52878ed
compatibility::simple-validator-upgrade::liveness-check : committed: 7989.1698618933 txn/s, latency: 3645.400541461337 ms, (p50: 2800 ms, p90: 3900 ms, p99: 28400 ms), latency samples: 328740
2. Upgrading first Validator to new version: 25421b3195d4d779277f4f23e6fab0a357e98719
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 2463.802855385315 txn/s, latency: 12528.691370262392 ms, (p50: 14500 ms, p90: 16600 ms, p99: 19000 ms), latency samples: 102900
3. Upgrading rest of first batch to new version: 25421b3195d4d779277f4f23e6fab0a357e98719
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3280.7305749804323 txn/s, latency: 9362.441756272401 ms, (p50: 9200 ms, p90: 14000 ms, p99: 14300 ms), latency samples: 139500
4. upgrading second batch to new version: 25421b3195d4d779277f4f23e6fab0a357e98719
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6433.034762528811 txn/s, latency: 5098.827052303429 ms, (p50: 4800 ms, p90: 8400 ms, p99: 9500 ms), latency samples: 230960
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 25421b3195d4d779277f4f23e6fab0a357e98719 passed
Test Ok

This comment has been minimized.

Copy link
Contributor

✅ Forge suite framework_upgrade success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 25421b3195d4d779277f4f23e6fab0a357e98719

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 25421b3195d4d779277f4f23e6fab0a357e98719 (PR)
Upgrade the nodes to version: 25421b3195d4d779277f4f23e6fab0a357e98719
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1164.4652023919389 txn/s, submitted: 1167.2887187444753 txn/s, failed submission: 2.823516352536526 txn/s, expired: 2.823516352536526 txn/s, latency: 2680.367276217418 ms, (p50: 2100 ms, p90: 4800 ms, p99: 9400 ms), latency samples: 98980
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1139.0209534738538 txn/s, submitted: 1141.0927377465428 txn/s, failed submission: 2.0717842726889013 txn/s, expired: 2.0717842726889013 txn/s, latency: 2770.05422392886 ms, (p50: 2100 ms, p90: 4800 ms, p99: 9300 ms), latency samples: 98960
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 25421b3195d4d779277f4f23e6fab0a357e98719 passed
Upgrade the remaining nodes to version: 25421b3195d4d779277f4f23e6fab0a357e98719
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1157.3417725137012 txn/s, submitted: 1159.5412057567908 txn/s, failed submission: 2.199433243089512 txn/s, expired: 2.199433243089512 txn/s, latency: 2718.1535632839223 ms, (p50: 2100 ms, p90: 4800 ms, p99: 9500 ms), latency samples: 105240
Test Ok

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 25421b3195d4d779277f4f23e6fab0a357e98719

two traffics test: inner traffic : committed: 8712.888672797464 txn/s, latency: 4496.209994318996 ms, (p50: 4400 ms, p90: 5100 ms, p99: 10300 ms), latency samples: 3766940
two traffics test : committed: 100.0401285884577 txn/s, latency: 2103.5054945054944 ms, (p50: 2000 ms, p90: 2400 ms, p99: 5000 ms), latency samples: 1820
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.217, avg: 0.211", "QsPosToProposal: max: 0.267, avg: 0.244", "ConsensusProposalToOrdered: max: 0.307, avg: 0.287", "ConsensusOrderedToCommit: max: 0.366, avg: 0.354", "ConsensusProposalToCommit: max: 0.650, avg: 0.640"]
Max round gap was 1 [limit 4] at version 1823612. Max no progress secs was 4.944645 [limit 15] at version 1823612.
Test Ok

@zjma zjma merged commit cc9d824 into main Jun 23, 2024
43 of 46 checks passed
@zjma zjma deleted the api_fix branch June 23, 2024 05:49
@zjma
Copy link
Contributor Author

zjma commented Jun 23, 2024

  1. update the transaction example that a new field is added to API dkg_transcript when the validator transaction type is dkg_result. It isn't very clear to understand the example..
  2. the changes seem to be in the API response but in the description, it's said omitted. You may want to update the example accordingly. The events look good.

Example:

image

@larry-aptos I think i didn't want to give full example in PR description because it's too long. Perhaps I meant "detail omitted..."

Comment on lines +234 to +236
aptos_types::transaction::Transaction::ValidatorTransaction(txn) => {
Transaction::ValidatorTransaction((txn, info, events, timestamp).into())
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bookmark

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants