Event V2 Translation #14615

junkil-park · 2024-09-12T20:36:46Z

Description

This PR introduces the Event V2 Translation Engine, a framework for translating well-known V2 events into V1 format. So, the Node APIs can output those events in the V1 format.
Added the new flag enable_event_v2_translation to the node config for feature-gating.
Added the event translators for the account and coin module events, translating:
- coin::CoinDeposit -> coin::DepositEvent
- coin::CoinWithdraw -> coin::WithdrawEvent
- account::CoinRegister -> account::CoinRegisterEvent
- account::KeyRotation -> account::KeyRotationEvent
Added the event translators for token objects and token v1 module events as well.
Implemented the e2e API test environment where we can simulate and test the module event migration. And, added the e2e tests for the event transaction for the account and coin modules.

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Tested on the localnet.
Added various e2e API tests.

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

trunk-io · 2024-09-12T20:36:49Z

⏱️ 2h 42m total CI duration on this PR

Slowest 15 Jobs	Cumulative Duration	Recent Runs
general-lints	20m	🟩 🟩 🟩 🟩 🟩 (+7 more)
rust-cargo-deny	18m	🟩 🟩 🟩 🟩 🟩 (+7 more)
check-dynamic-deps	13m	🟩 🟩 🟩 🟩 🟩 (+7 more)
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	6m	⬜
semgrep/ci	5m	🟩 🟩 🟩 🟩 🟩 (+7 more)

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

storage/aptosdb/src/db/include/aptosdb_reader.rs

storage/indexer/src/db_indexer.rs

storage/indexer_schemas/src/schema/translated_v1_event/mod.rs

storage/indexer/src/db_indexer.rs

api/src/context.rs

storage/indexer/src/event_v2_translator.rs

grao1991 · 2024-12-02T23:13:47Z

storage/indexer/src/event_v2_translator.rs

+        if let Some(seq) = self.get_cached_sequence_number(event_key) {
+            Ok(seq + 1)
+        } else {
+            let seq = self


in what case you will find the data in database but not in cache?

The cache and the database don't need to be perfectly synced as we don't plan to support the fast sync at the moment. When a node stops and resumes, we don't fill the cache with the data in the DB, and so the data may be present in the DB but not in the cache.

I mean, what's wrong with loading it fully on startup? seems more straightforward

If we fully load it on startup, memory usage would continuously increase even after restarting the node. If we decide to support fast sync in the future, fully loading at startup would become necessary. At that point, I can revisit and update the code to accommodate that requirement.

msmouse · 2024-12-03T18:38:54Z

api/src/context.rs

+                if self.indexer_reader.is_some()
+                    && self
+                        .node_config
+                        .indexer_db_config
+                        .enable_event_v2_translation
+                {
+                    self.translate_v2_to_v1_events_for_version(txn.version, &mut txn.events)
+                        .ok();
+                }
+                Ok(txn)


nit: Can we reduce the boilerplate a little by introducing
fn maybe_transalate_v2_events_to_v1(&self, txn: TransactionOnChainDate) -> TransactionOnChainData ?

Thank you for your comments. I introduced maybe_translate_v2_to_v1_events and refactored the code accordingly.

msmouse · 2024-12-03T19:10:02Z

storage/indexer/src/event_v2_translator.rs

+        let mut translators: HashMap<TypeTag, Box<dyn EventV2Translator + Send + Sync>> =
+            HashMap::new();
+        translators.insert(COIN_DEPOSIT_TYPE.clone(), Box::new(CoinDepositTranslator));
+        translators.insert(COIN_WITHDRAW_TYPE.clone(), Box::new(CoinWithdrawTranslator));


nit: can't we do a literal array and build the hashmap from there?

I updated it, building the hashmap from an array.

msmouse · 2024-12-03T19:23:00Z

storage/indexer/src/event_v2_translator.rs

+        if let Some(seq) = self.get_cached_sequence_number(event_key) {
+            Ok(seq + 1)
+        } else {
+            let seq = self


I mean, what's wrong with loading it fully on startup? seems more straightforward

msmouse · 2024-12-03T19:28:15Z

storage/indexer/src/event_v2_translator.rs

+        let group_tag_str = "0x1::object::ObjectGroup".to_string();
+        let group_tag = StructTag::from_str(&group_tag_str)?;


ObjectGroupResource::struct_tag()

also maybe worth caching it?

stretch: add StateKey::resource_group_typed::<MoveStruct>(address)

I updated the code using ObjectGroupResource::struct_tag() and caching it.

msmouse · 2024-12-04T01:33:16Z

storage/indexer/src/event_v2_translator.rs

+
+impl EventV2TranslationEngine {
+    pub fn new(main_db_reader: Arc<dyn DbReader>, internal_indexer_db: Arc<DB>) -> Self {
+        let translators: HashMap<TypeTag, Box<dyn EventV2Translator + Send + Sync>> = [


well.. the .clone and Box::new can be done in a map(), no?

(Guess it's too messy to type the translator constructor, I'm gonna approve)

Yeah, it does seem tricky to get the typing to work...

github-actions · 2024-12-05T02:19:36Z

✅ Forge suite `realistic_env_max_load` success on `c9a36c51c59c6d16f9a1c107bc4ebd193311c97e`

two traffics test: inner traffic : committed: 14720.53 txn/s, latency: 2698.39 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3200 ms), latency samples: 5597040
two traffics test : committed: 100.08 txn/s, latency: 1423.03 ms, (p50: 1400 ms, p70: 1400, p90: 1500 ms, p99: 2400 ms), latency samples: 1780
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.564, avg: 1.494", "ConsensusProposalToOrdered: max: 0.318, avg: 0.292", "ConsensusOrderedToCommit: max: 0.378, avg: 0.368", "ConsensusProposalToCommit: max: 0.667, avg: 0.660"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.76s no progress at version 22531 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.65s no progress at version 2848408 (avg 0.65s) [limit 16].
Test Ok

github-actions · 2024-12-05T02:21:04Z

✅ Forge suite `framework_upgrade` success on `c2969f0b0ecd826aa1516a386aed59496a247958` ==> `c9a36c51c59c6d16f9a1c107bc4ebd193311c97e`

Compatibility test results for c2969f0b0ecd826aa1516a386aed59496a247958 ==> c9a36c51c59c6d16f9a1c107bc4ebd193311c97e (PR)
Upgrade the nodes to version: c9a36c51c59c6d16f9a1c107bc4ebd193311c97e
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1517.07 txn/s, submitted: 1519.06 txn/s, failed submission: 1.99 txn/s, expired: 1.99 txn/s, latency: 2270.44 ms, (p50: 2100 ms, p70: 2400, p90: 3600 ms, p99: 5100 ms), latency samples: 121840
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1314.45 txn/s, submitted: 1318.20 txn/s, failed submission: 3.75 txn/s, expired: 3.75 txn/s, latency: 2236.94 ms, (p50: 2100 ms, p70: 2400, p90: 3200 ms, p99: 5100 ms), latency samples: 119140
5. check swarm health
Compatibility test for c2969f0b0ecd826aa1516a386aed59496a247958 ==> c9a36c51c59c6d16f9a1c107bc4ebd193311c97e passed
Upgrade the remaining nodes to version: c9a36c51c59c6d16f9a1c107bc4ebd193311c97e
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1274.98 txn/s, submitted: 1278.28 txn/s, failed submission: 3.30 txn/s, expired: 3.30 txn/s, latency: 2310.43 ms, (p50: 2100 ms, p70: 2400, p90: 3400 ms, p99: 5100 ms), latency samples: 115820
Test Ok

github-actions · 2024-12-05T02:21:31Z

✅ Forge suite `compat` success on `c2969f0b0ecd826aa1516a386aed59496a247958` ==> `c9a36c51c59c6d16f9a1c107bc4ebd193311c97e`

Compatibility test results for c2969f0b0ecd826aa1516a386aed59496a247958 ==> c9a36c51c59c6d16f9a1c107bc4ebd193311c97e (PR)
1. Check liveness of validators at old version: c2969f0b0ecd826aa1516a386aed59496a247958
compatibility::simple-validator-upgrade::liveness-check : committed: 16678.74 txn/s, latency: 1991.31 ms, (p50: 1900 ms, p70: 2100, p90: 2500 ms, p99: 3900 ms), latency samples: 556260
2. Upgrading first Validator to new version: c9a36c51c59c6d16f9a1c107bc4ebd193311c97e
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7376.42 txn/s, latency: 3873.55 ms, (p50: 4400 ms, p70: 4600, p90: 4700 ms, p99: 4700 ms), latency samples: 136600
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7310.87 txn/s, latency: 4425.11 ms, (p50: 4700 ms, p70: 4800, p90: 4800 ms, p99: 4900 ms), latency samples: 251360
3. Upgrading rest of first batch to new version: c9a36c51c59c6d16f9a1c107bc4ebd193311c97e
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6427.89 txn/s, latency: 4416.53 ms, (p50: 5100 ms, p70: 5300, p90: 5400 ms, p99: 5500 ms), latency samples: 116440
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6397.99 txn/s, latency: 5191.20 ms, (p50: 5700 ms, p70: 5800, p90: 5900 ms, p99: 6100 ms), latency samples: 220020
4. upgrading second batch to new version: c9a36c51c59c6d16f9a1c107bc4ebd193311c97e
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 1336.48 txn/s, latency: 16893.56 ms, (p50: 17700 ms, p70: 23800, p90: 27500 ms, p99: 28600 ms), latency samples: 56300
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2116.04 txn/s, latency: 15774.42 ms, (p50: 17200 ms, p70: 20200, p90: 25100 ms, p99: 28300 ms), latency samples: 77220
5. check swarm health
Compatibility test for c2969f0b0ecd826aa1516a386aed59496a247958 ==> c9a36c51c59c6d16f9a1c107bc4ebd193311c97e passed
Test Ok

junkil-park force-pushed the jpark/event-v2-translation branch 9 times, most recently from 50b9c0f to 832b754 Compare September 16, 2024 16:22

grao1991 previously requested changes Sep 16, 2024

View reviewed changes

junkil-park force-pushed the jpark/event-v2-translation branch 4 times, most recently from 80c5021 to bfbe123 Compare September 24, 2024 03:46

junkil-park force-pushed the jpark/event-v2-translation branch 10 times, most recently from 123fbfe to 27650b3 Compare October 3, 2024 16:15

junkil-park force-pushed the jpark/event-v2-translation branch from 27650b3 to c202506 Compare October 4, 2024 16:52

junkil-park force-pushed the jpark/event-v2-translation branch 2 times, most recently from ccd6843 to c38fc96 Compare October 16, 2024 11:27

junkil-park force-pushed the jpark/event-v2-translation branch 2 times, most recently from 32088d7 to 523e22f Compare October 23, 2024 07:11

junkil-park force-pushed the jpark/event-v2-translation branch 2 times, most recently from b4c76bf to ea0bbbd Compare December 2, 2024 20:37

grao1991 reviewed Dec 2, 2024

View reviewed changes

junkil-park force-pushed the jpark/event-v2-translation branch from ea0bbbd to 660a1ab Compare December 3, 2024 00:10

msmouse reviewed Dec 3, 2024

View reviewed changes

junkil-park force-pushed the jpark/event-v2-translation branch from 660a1ab to f52c1bc Compare December 4, 2024 01:09

msmouse approved these changes Dec 4, 2024

View reviewed changes

junkil-park force-pushed the jpark/event-v2-translation branch 3 times, most recently from d137eea to a8d65f4 Compare December 4, 2024 19:14

junkil-park enabled auto-merge (squash) December 5, 2024 01:12

This comment has been minimized.

Sign in to view

Event V2 Translation

c9a36c5

junkil-park force-pushed the jpark/event-v2-translation branch from a8d65f4 to c9a36c5 Compare December 5, 2024 01:54

This comment has been minimized.

Sign in to view

junkil-park merged commit 9e5cc9d into main Dec 5, 2024
48 checks passed

junkil-park deleted the jpark/event-v2-translation branch December 5, 2024 02:25

junkil-park added a commit that referenced this pull request Dec 5, 2024

Event V2 Translation (#14615)

6b0b38c

junkil-park mentioned this pull request Dec 5, 2024

Cherry-pick: Event V2 Translation (#14615) #15515

Merged

junkil-park added a commit that referenced this pull request Dec 5, 2024

Event V2 Translation (#14615) (#15515)

17f4b41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Event V2 Translation #14615

Event V2 Translation #14615

junkil-park commented Sep 12, 2024 •

edited

Loading

trunk-io bot commented Sep 12, 2024 •

edited

Loading

grao1991 Dec 2, 2024

junkil-park Dec 2, 2024 •

edited

Loading

msmouse Dec 3, 2024

junkil-park Dec 4, 2024

msmouse Dec 3, 2024

junkil-park Dec 4, 2024

msmouse Dec 3, 2024

junkil-park Dec 4, 2024

msmouse Dec 3, 2024

msmouse Dec 3, 2024 •

edited

Loading

junkil-park Dec 4, 2024 •

edited

Loading

msmouse Dec 4, 2024

junkil-park Dec 4, 2024

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

		let group_tag_str = "0x1::object::ObjectGroup".to_string();
		let group_tag = StructTag::from_str(&group_tag_str)?;

Event V2 Translation #14615

Event V2 Translation #14615

Conversation

junkil-park commented Sep 12, 2024 • edited Loading

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

trunk-io bot commented Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

junkil-park Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msmouse Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

junkil-park Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Dec 5, 2024

✅ Forge suite realistic_env_max_load success on c9a36c51c59c6d16f9a1c107bc4ebd193311c97e

github-actions bot commented Dec 5, 2024

✅ Forge suite framework_upgrade success on c2969f0b0ecd826aa1516a386aed59496a247958 ==> c9a36c51c59c6d16f9a1c107bc4ebd193311c97e

github-actions bot commented Dec 5, 2024

✅ Forge suite compat success on c2969f0b0ecd826aa1516a386aed59496a247958 ==> c9a36c51c59c6d16f9a1c107bc4ebd193311c97e

junkil-park commented Sep 12, 2024 •

edited

Loading

trunk-io bot commented Sep 12, 2024 •

edited

Loading

junkil-park Dec 2, 2024 •

edited

Loading

msmouse Dec 3, 2024 •

edited

Loading

junkil-park Dec 4, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `c9a36c51c59c6d16f9a1c107bc4ebd193311c97e`

✅ Forge suite `framework_upgrade` success on `c2969f0b0ecd826aa1516a386aed59496a247958` ==> `c9a36c51c59c6d16f9a1c107bc4ebd193311c97e`

✅ Forge suite `compat` success on `c2969f0b0ecd826aa1516a386aed59496a247958` ==> `c9a36c51c59c6d16f9a1c107bc4ebd193311c97e`