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

[Protobuf] Support validator transaction type in protobuf #13897

Merged
merged 6 commits into from
Jul 10, 2024

Conversation

larry-aptos
Copy link
Contributor

@larry-aptos larry-aptos commented Jul 2, 2024

Description

  • Support validator transaction at protobuf
  • Also remove the unused bigquery transaction type.

More context: #13718

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) Indexer

How Has This Been Tested?

Lint + ut

more to add

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 Jul 2, 2024

⏱️ 3h 9m total CI duration on this PR
Job Cumulative Duration Recent Runs
test-fuzzers 1h 49m 🟩🟩🟩
rust-move-tests 14m 🟩
forge-e2e-test / forge 14m 🟩
rust-move-tests 12m 🟩
rust-move-tests 10m 🟩
rust-move-tests 10m 🟩
general-lints 7m 🟩🟩🟩🟩
rust-move-tests 6m 🟥
check-dynamic-deps 4m 🟩🟩🟩🟩
semgrep/ci 2m 🟩🟩🟩🟩
file_change_determinator 44s 🟩🟩🟩🟩
file_change_determinator 43s 🟩🟩🟩🟩
permission-check 12s 🟩🟩🟩🟩
permission-check 11s 🟩🟩🟩🟩
permission-check 10s 🟩🟩🟩🟩
permission-check 8s 🟩🟩🟩🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

@@ -690,9 +697,9 @@ impl From<QuorumCertifiedUpdate> for ExportedQuorumCertifiedUpdate {
/// A more API-friendly representation of the on-chain `aptos_types::aggregate_signature::AggregateSignature`.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Object)]
pub struct ExportedAggregateSignature {
signer_indices: Vec<usize>,
Copy link
Contributor Author

@larry-aptos larry-aptos Jul 3, 2024

Choose a reason for hiding this comment

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

only the visibility change below in this file.

@@ -10,7 +10,7 @@ import "aptos/transaction/v1/transaction.proto";
// This is for storage only.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Proto format

@larry-aptos larry-aptos changed the title format and add new transaction type. [Protobuf] Support validator transaction type in protobuf Jul 3, 2024
@larry-aptos larry-aptos requested a review from a team July 3, 2024 00:41
@@ -1,6 +1,5 @@
/* eslint-disable */

export * as bigquery_schema from "./index.aptos.bigquery_schema";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove this; we have parquet effort going on.

Copy link
Contributor

@rtso rtso left a comment

Choose a reason for hiding this comment

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

How do you plan to test this change?

@@ -621,6 +621,13 @@ impl ValidatorTransaction {
ValidatorTransaction::DkgResult(t) => t.timestamp,
}
}

pub fn events(&self) -> &[Event] {
Copy link
Contributor

Choose a reason for hiding this comment

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

To future proof, we should include the write sets too. There's nothing to index in it now, but there could be in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is handled in TransactionInfo parsing, which is shared among all transaction types.

Comment on lines +134 to +139
message DkgTranscript {
uint64 epoch = 1;
string author = 2;
bytes payload = 3;
}
DkgTranscript dkg_transcript = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

qq: how do you know these are the only fields needed for DkgUpdate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copied from exported dkg output.

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Object)]
pub struct ExportedDKGTranscript {
    epoch: U64,
    author: Address,
    payload: HexEncodedBytes,
}

@larry-aptos
Copy link
Contributor Author

https://gist.github.com/larry-aptos/bfb876716dc617c5717c312d2f7c8255 tested locally and it looks good and can see all the events.

example

{
            "key": {
              "creationNumber": "12",
              "accountAddress": "0x7da5c64f4009077729b9699f46b36fd7e3567117c50b3de6e56bc7b443c9e440"
            },
            "type": {
              "type": "MOVE_TYPES_STRUCT",
              "struct": {
                "address": "0x1",
                "module": "stake",
                "name": "DistributeRewardsEvent"
              }
            },
            "data": "{\"pool_address\":\"0x7da5c64f4009077729b9699f46b36fd7e3567117c50b3de6e56bc7b443c9e440\",\"rewards_amount\":\"0\"}",
            "typeStr": "0x1::stake::DistributeRewardsEvent"
          },
          ```

@larry-aptos larry-aptos enabled auto-merge (squash) July 9, 2024 20:52

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.

Copy link
Contributor

github-actions bot commented Jul 9, 2024

✅ Forge suite compat success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 9662384a703bcdd0e71d06d14a6101a13cc578a8

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 9662384a703bcdd0e71d06d14a6101a13cc578a8 (PR)
1. Check liveness of validators at old version: 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5
compatibility::simple-validator-upgrade::liveness-check : committed: 6463.7017609500335 txn/s, latency: 4168.722452777192 ms, (p50: 3300 ms, p90: 5800 ms, p99: 19600 ms), latency samples: 284820
2. Upgrading first Validator to new version: 9662384a703bcdd0e71d06d14a6101a13cc578a8
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 3345.99089959956 txn/s, latency: 7179.101060572928 ms, (p50: 8500 ms, p90: 9800 ms, p99: 12100 ms), latency samples: 91460
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3325.7609766028936 txn/s, latency: 9350.47858097076 ms, (p50: 9500 ms, p90: 14800 ms, p99: 15100 ms), latency samples: 134740
3. Upgrading rest of first batch to new version: 9662384a703bcdd0e71d06d14a6101a13cc578a8
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 3093.235987143695 txn/s, latency: 8377.5786966046 ms, (p50: 10000 ms, p90: 12400 ms, p99: 13000 ms), latency samples: 73040
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3216.4207419096647 txn/s, latency: 9631.10809530749 ms, (p50: 9400 ms, p90: 15100 ms, p99: 15400 ms), latency samples: 137240
4. upgrading second batch to new version: 9662384a703bcdd0e71d06d14a6101a13cc578a8
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 6841.701443265394 txn/s, latency: 4099.479409183972 ms, (p50: 4800 ms, p90: 5100 ms, p99: 5300 ms), latency samples: 136760
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 5800.88154053689 txn/s, latency: 5556.6776367928505 ms, (p50: 5100 ms, p90: 9400 ms, p99: 10800 ms), latency samples: 237220
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 9662384a703bcdd0e71d06d14a6101a13cc578a8 passed
Test Ok

Copy link
Contributor

github-actions bot commented Jul 9, 2024

✅ Forge suite framework_upgrade success on 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 9662384a703bcdd0e71d06d14a6101a13cc578a8

Compatibility test results for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 9662384a703bcdd0e71d06d14a6101a13cc578a8 (PR)
Upgrade the nodes to version: 9662384a703bcdd0e71d06d14a6101a13cc578a8
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1056.789523409784 txn/s, submitted: 1059.7382814703822 txn/s, failed submission: 2.948758060598238 txn/s, expired: 2.948758060598238 txn/s, latency: 3189.907136724619 ms, (p50: 2300 ms, p90: 6900 ms, p99: 11200 ms), latency samples: 93180
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1118.4787082154583 txn/s, submitted: 1120.0765349414805 txn/s, failed submission: 1.5978267260220833 txn/s, expired: 1.5978267260220833 txn/s, latency: 2781.9681836734694 ms, (p50: 2100 ms, p90: 4800 ms, p99: 9000 ms), latency samples: 98000
5. check swarm health
Compatibility test for 1c2ee7082d6eff8c811ee25d6f5a7d00860a75d5 ==> 9662384a703bcdd0e71d06d14a6101a13cc578a8 passed
Upgrade the remaining nodes to version: 9662384a703bcdd0e71d06d14a6101a13cc578a8
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1097.799606323097 txn/s, submitted: 1099.4109708113044 txn/s, failed submission: 1.611364488207523 txn/s, expired: 1.611364488207523 txn/s, latency: 3043.49963304676 ms, (p50: 2400 ms, p90: 5400 ms, p99: 12300 ms), latency samples: 95380
Test Ok

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 9662384a703bcdd0e71d06d14a6101a13cc578a8

two traffics test: inner traffic : committed: 8474.65497588383 txn/s, latency: 4620.771722562224 ms, (p50: 4500 ms, p90: 5900 ms, p99: 10500 ms), latency samples: 3663380
two traffics test : committed: 99.94473236556843 txn/s, latency: 2169.023033707865 ms, (p50: 1900 ms, p90: 2600 ms, p99: 8700 ms), latency samples: 1780
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.216, avg: 0.211", "QsPosToProposal: max: 0.192, avg: 0.183", "ConsensusProposalToOrdered: max: 0.306, avg: 0.290", "ConsensusOrderedToCommit: max: 0.370, avg: 0.361", "ConsensusProposalToCommit: max: 0.658, avg: 0.651"]
Max round gap was 1 [limit 4] at version 1628161. Max no progress secs was 5.150338 [limit 15] at version 1628161.
Test Ok

@larry-aptos larry-aptos merged commit 5c48aee into main Jul 10, 2024
44 of 47 checks passed
@larry-aptos larry-aptos deleted the fix-the-validator-thing branch July 10, 2024 22:57
larry-aptos added a commit that referenced this pull request Jul 10, 2024
* format and add new transaction type.

* format  + cleanup + new transaciton type.

* update.

* fix the validator transaction.

* upgrade protobuf.

* grpc support validator transaction.
larry-aptos added a commit that referenced this pull request Jul 10, 2024
* format and add new transaction type.

* format  + cleanup + new transaciton type.

* update.

* fix the validator transaction.

* upgrade protobuf.

* grpc support validator transaction.
larry-aptos added a commit that referenced this pull request Jul 11, 2024
* format and add new transaction type.

* format  + cleanup + new transaciton type.

* update.

* fix the validator transaction.

* upgrade protobuf.

* grpc support validator transaction.
zi0Black pushed a commit that referenced this pull request Jul 12, 2024
* format and add new transaction type.

* format  + cleanup + new transaciton type.

* update.

* fix the validator transaction.

* upgrade protobuf.

* grpc support validator transaction.
larry-aptos added a commit that referenced this pull request Jul 16, 2024
* format and add new transaction type.

* format  + cleanup + new transaciton type.

* update.

* fix the validator transaction.

* upgrade protobuf.

* grpc support validator transaction.
ying-w pushed a commit that referenced this pull request Jul 16, 2024
* format and add new transaction type.

* format  + cleanup + new transaciton type.

* update.

* fix the validator transaction.

* upgrade protobuf.

* grpc support validator transaction.
sherry-x pushed a commit that referenced this pull request Jul 16, 2024
…14019)

* format and add new transaction type.

* format  + cleanup + new transaciton type.

* update.

* fix the validator transaction.

* upgrade protobuf.

* grpc support validator transaction.

Co-authored-by: larry-aptos <112209412+larry-aptos@users.noreply.github.com>
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