From 1906da1c26336beae67565e5a24f1744b6e50096 Mon Sep 17 00:00:00 2001 From: Satya Vusirikala Date: Tue, 14 Jan 2025 13:25:36 -0800 Subject: [PATCH] Support orderless transactions in Indexer --- hasura-api/metadata-json/unified.json | 3 +- .../metadata-json/unified_transition.json | 3 +- python/utils/transaction_utils.py | 3 +- rust/Cargo.lock | 18 +++++- rust/Cargo.toml | 2 +- .../src/models/user_transactions_models.rs | 2 + .../user_transaction_processor_tests.rs | 1 + .../parquet_user_transactions.rs | 14 ++++- .../2022-08-08-043603_core_tables/up.sql | 6 +- .../postgres/models/events_models/events.rs | 1 + .../models/token_models/token_activities.rs | 1 + .../user_transactions.rs | 14 ++++- rust/processor/src/db/postgres/schema.rs | 1 + rust/processor/src/transaction_filter.rs | 2 + rust/processor/src/utils/util.rs | 59 +++++++++++++++---- .../db/common/models/events_models/events.rs | 1 + rust/testing-transactions/src/lib.rs | 2 + 17 files changed, 111 insertions(+), 22 deletions(-) diff --git a/hasura-api/metadata-json/unified.json b/hasura-api/metadata-json/unified.json index 40fea42f0..30ccaf100 100644 --- a/hasura-api/metadata-json/unified.json +++ b/hasura-api/metadata-json/unified.json @@ -2335,7 +2335,8 @@ "sender", "sequence_number", "timestamp", - "version" + "version", + "replay_protection_nonce" ], "filter": {}, "limit": 100 diff --git a/hasura-api/metadata-json/unified_transition.json b/hasura-api/metadata-json/unified_transition.json index 27b151bcb..aeef5bd1d 100644 --- a/hasura-api/metadata-json/unified_transition.json +++ b/hasura-api/metadata-json/unified_transition.json @@ -2384,7 +2384,8 @@ "sender", "sequence_number", "timestamp", - "version" + "version", + "replay_protection_nonce" ], "filter": {}, "limit": 100 diff --git a/python/utils/transaction_utils.py b/python/utils/transaction_utils.py index e759dfdbe..41073f832 100644 --- a/python/utils/transaction_utils.py +++ b/python/utils/transaction_utils.py @@ -31,7 +31,8 @@ def get_sender( user_transaction_request = get_user_transaction_request(user_transaction) return user_transaction_request.sender - +# Question: Multisig transactions could also have an entry function payload inside them. +# Is it okay to ignore that entry function payload here? def get_entry_function_payload( user_transaction: transaction_pb2.UserTransaction, ) -> transaction_pb2.EntryFunctionPayload: diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 4ff913872..b5e31dddc 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -318,6 +318,18 @@ dependencies = [ "tonic 0.11.0", ] +[[package]] +name = "aptos-protos" +version = "1.3.1" +source = "git+https://github.com/aptos-labs/aptos-core.git?rev=f3bbed37ea0fbd31c6ad56361724247e1a417d9b#f3bbed37ea0fbd31c6ad56361724247e1a417d9b" +dependencies = [ + "futures-core", + "pbjson", + "prost 0.12.6", + "serde", + "tonic 0.11.0", +] + [[package]] name = "aptos-system-utils" version = "0.1.0" @@ -2286,7 +2298,7 @@ dependencies = [ "aptos-indexer-processor-sdk", "aptos-indexer-test-transactions", "aptos-indexer-testing-framework", - "aptos-protos 1.3.1 (git+https://github.com/aptos-labs/aptos-core.git?rev=5c48aee129b5a141be2792ffa3d9bd0a1a61c9cb)", + "aptos-protos 1.3.1 (git+https://github.com/aptos-labs/aptos-core.git?rev=f3bbed37ea0fbd31c6ad56361724247e1a417d9b)", "assert-json-diff", "bigdecimal", "chrono", @@ -3377,7 +3389,7 @@ dependencies = [ "allocative_derive", "anyhow", "aptos-moving-average 0.1.0", - "aptos-protos 1.3.1 (git+https://github.com/aptos-labs/aptos-core.git?rev=5c48aee129b5a141be2792ffa3d9bd0a1a61c9cb)", + "aptos-protos 1.3.1 (git+https://github.com/aptos-labs/aptos-core.git?rev=f3bbed37ea0fbd31c6ad56361724247e1a417d9b)", "async-trait", "bcs", "bigdecimal", @@ -4729,7 +4741,7 @@ dependencies = [ name = "testing-transactions" version = "0.1.0" dependencies = [ - "aptos-protos 1.3.1 (git+https://github.com/aptos-labs/aptos-core.git?rev=5c48aee129b5a141be2792ffa3d9bd0a1a61c9cb)", + "aptos-protos 1.3.1 (git+https://github.com/aptos-labs/aptos-core.git?rev=f3bbed37ea0fbd31c6ad56361724247e1a417d9b)", "serde_json", ] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 72c748f9c..033fc8903 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -32,7 +32,7 @@ ahash = { version = "0.8.7", features = ["serde"] } anyhow = "1.0.86" aptos-indexer-processor-sdk = { git = "https://github.com/aptos-labs/aptos-indexer-processor-sdk.git", rev = "e6867c50a2c30ef16ad6f82e02313b2ba5ce361a" } aptos-indexer-processor-sdk-server-framework = { git = "https://github.com/aptos-labs/aptos-indexer-processor-sdk.git", rev = "e6867c50a2c30ef16ad6f82e02313b2ba5ce361a" } -aptos-protos = { git = "https://github.com/aptos-labs/aptos-core.git", rev = "5c48aee129b5a141be2792ffa3d9bd0a1a61c9cb" } +aptos-protos = { git = "https://github.com/aptos-labs/aptos-core.git", rev = "f3bbed37ea0fbd31c6ad56361724247e1a417d9b" } aptos-system-utils = { git = "https://github.com/aptos-labs/aptos-core.git", rev = "202bdccff2b2d333a385ae86a4fcf23e89da9f62" } aptos-indexer-test-transactions = { git = "https://github.com/aptos-labs/aptos-core.git", rev = "8bb628129ff48241c650178caf1ff8bf53a44e5e" } aptos-indexer-testing-framework = { git = "https://github.com/aptos-labs/aptos-indexer-processor-sdk.git", rev = "e6867c50a2c30ef16ad6f82e02313b2ba5ce361a" } diff --git a/rust/integration-tests/src/models/user_transactions_models.rs b/rust/integration-tests/src/models/user_transactions_models.rs index d474babe0..aa2c1d373 100644 --- a/rust/integration-tests/src/models/user_transactions_models.rs +++ b/rust/integration-tests/src/models/user_transactions_models.rs @@ -51,4 +51,6 @@ pub struct UserTransaction { pub entry_function_contract_address: Option, pub entry_function_module_name: Option, pub entry_function_function_name: Option, + // Question: Is it okay to use i64 here, even though replay_protection_nonce is defined as u64 in aptos-core? + pub replay_protection_nonce: Option, } diff --git a/rust/integration-tests/src/sdk_tests/user_transaction_processor_tests.rs b/rust/integration-tests/src/sdk_tests/user_transaction_processor_tests.rs index 70a7986d0..22bdc5b72 100644 --- a/rust/integration-tests/src/sdk_tests/user_transaction_processor_tests.rs +++ b/rust/integration-tests/src/sdk_tests/user_transaction_processor_tests.rs @@ -48,6 +48,7 @@ mod sdk_user_txn_processor_tests { run_processor_test, setup_test_environment, validate_json, DEFAULT_OUTPUT_FOLDER, }, }; + // TODO: Add some orderless transactions here use aptos_indexer_test_transactions::{ IMPORTED_MAINNET_TXNS_1803170308_USER_TXN_MULTI_KEY_KEYLESS, IMPORTED_MAINNET_TXNS_2175935_USER_TXN_MULTI_ED25519, diff --git a/rust/processor/src/db/parquet/models/user_transaction_models/parquet_user_transactions.rs b/rust/processor/src/db/parquet/models/user_transaction_models/parquet_user_transactions.rs index 4adeb71ca..0b0587c40 100644 --- a/rust/processor/src/db/parquet/models/user_transaction_models/parquet_user_transactions.rs +++ b/rust/processor/src/db/parquet/models/user_transaction_models/parquet_user_transactions.rs @@ -17,7 +17,7 @@ use allocative::Allocative; use anyhow::Result; use aptos_protos::{ transaction::v1::{ - TransactionInfo, UserTransaction as UserTransactionPB, UserTransactionRequest, + transaction_payload::ExtraConfig, ExtraConfigV1, TransactionInfo, UserTransaction as UserTransactionPB, UserTransactionRequest }, util::timestamp::Timestamp, }; @@ -43,6 +43,8 @@ pub struct UserTransaction { pub storage_refund_octa: u64, pub is_transaction_success: bool, pub num_signatures: i64, + // Question: Is it okay to use u64 here? + pub replay_protection_nonce: Option, } impl NamedTable for UserTransaction { @@ -112,6 +114,16 @@ impl UserTransaction { .map(|fs| fs.storage_fee_refund_octas) .unwrap_or(0), num_signatures, + replay_protection_nonce: user_request.payload.as_ref() + .and_then(|payload| payload.extra_config.as_ref()) + .and_then(|extra_config| match extra_config { + ExtraConfig::ExtraConfigV1( + ExtraConfigV1 { + replay_protection_nonce, + multisig_address: _, + } + ) => replay_protection_nonce.clone() + }) } } diff --git a/rust/processor/src/db/postgres/migrations/2022-08-08-043603_core_tables/up.sql b/rust/processor/src/db/postgres/migrations/2022-08-08-043603_core_tables/up.sql index 7bc2c6caa..85e643044 100644 --- a/rust/processor/src/db/postgres/migrations/2022-08-08-043603_core_tables/up.sql +++ b/rust/processor/src/db/postgres/migrations/2022-08-08-043603_core_tables/up.sql @@ -163,13 +163,15 @@ CREATE TABLE user_transactions ( -- from UserTransaction "timestamp" TIMESTAMP NOT NULL, entry_function_id_str text NOT NULL, + replay_protection_nonce BIGINT, -- Default time columns inserted_at TIMESTAMP NOT NULL DEFAULT NOW(), -- Constraints CONSTRAINT fk_versions FOREIGN KEY (version) REFERENCES transactions (version), - UNIQUE (sender, sequence_number) + UNIQUE (sender, sequence_number, replay_protection_nonce) ); -CREATE INDEX ut_sender_seq_index ON user_transactions (sender, sequence_number); +/* Question: Is it okay to change this index by adding `replay_protection_nonce` in the key? */ +CREATE INDEX ut_sender_seq_index ON user_transactions (sender, sequence_number, replay_protection_nonce); CREATE INDEX ut_insat_index ON user_transactions (inserted_at); -- tracks signatures for user transactions CREATE TABLE signatures ( diff --git a/rust/processor/src/db/postgres/models/events_models/events.rs b/rust/processor/src/db/postgres/models/events_models/events.rs index d1e010484..aa4f8a2a4 100644 --- a/rust/processor/src/db/postgres/models/events_models/events.rs +++ b/rust/processor/src/db/postgres/models/events_models/events.rs @@ -11,6 +11,7 @@ use aptos_protos::transaction::v1::Event as EventPB; use field_count::FieldCount; use serde::{Deserialize, Serialize}; +// Question: Do we have to add a replay_protection_nonce here? #[derive(Clone, Debug, Deserialize, FieldCount, Identifiable, Insertable, Serialize)] #[diesel(primary_key(transaction_version, event_index))] #[diesel(table_name = events)] diff --git a/rust/processor/src/db/postgres/models/token_models/token_activities.rs b/rust/processor/src/db/postgres/models/token_models/token_activities.rs index cf2da00ac..01ac14ff3 100644 --- a/rust/processor/src/db/postgres/models/token_models/token_activities.rs +++ b/rust/processor/src/db/postgres/models/token_models/token_activities.rs @@ -30,6 +30,7 @@ pub struct TokenActivity { pub transaction_version: i64, pub event_account_address: String, pub event_creation_number: i64, + // Question: Do we need to add a replay_protection_nonce here? pub event_sequence_number: i64, pub token_data_id_hash: String, pub property_version: BigDecimal, diff --git a/rust/processor/src/db/postgres/models/user_transactions_models/user_transactions.rs b/rust/processor/src/db/postgres/models/user_transactions_models/user_transactions.rs index 40983a4b6..99bee9c0d 100644 --- a/rust/processor/src/db/postgres/models/user_transactions_models/user_transactions.rs +++ b/rust/processor/src/db/postgres/models/user_transactions_models/user_transactions.rs @@ -18,7 +18,7 @@ use crate::{ }, }; use aptos_protos::{ - transaction::v1::{UserTransaction as UserTransactionPB, UserTransactionRequest}, + transaction::v1::{transaction_payload::ExtraConfig, ExtraConfigV1, UserTransaction as UserTransactionPB, UserTransactionRequest}, util::timestamp::Timestamp, }; use bigdecimal::BigDecimal; @@ -43,6 +43,8 @@ pub struct UserTransaction { pub entry_function_contract_address: Option, pub entry_function_module_name: Option, pub entry_function_function_name: Option, + // Question: Is it okay to use i64 here, as the nonce is defined as u64 in aptos-core repo? + pub replay_protection_nonce: Option, } impl UserTransaction { @@ -96,6 +98,16 @@ impl UserTransaction { get_entry_function_function_name_from_user_request(user_request) .unwrap_or_default(), ), + replay_protection_nonce: user_request.payload.as_ref() + .and_then(|payload| payload.extra_config.as_ref()) + .and_then(|extra_config| match extra_config { + ExtraConfig::ExtraConfigV1( + ExtraConfigV1 { + replay_protection_nonce, + multisig_address: _, + } + ) => replay_protection_nonce.map(|nonce| nonce as i64) + }), }, Self::get_signatures(user_request, version, block_height), ) diff --git a/rust/processor/src/db/postgres/schema.rs b/rust/processor/src/db/postgres/schema.rs index 06a2b0947..b1b12f279 100644 --- a/rust/processor/src/db/postgres/schema.rs +++ b/rust/processor/src/db/postgres/schema.rs @@ -1271,6 +1271,7 @@ diesel::table! { entry_function_module_name -> Nullable, #[max_length = 255] entry_function_function_name -> Nullable, + replay_protection_nonce -> Nullable, } } diff --git a/rust/processor/src/transaction_filter.rs b/rust/processor/src/transaction_filter.rs index 39ae5204a..3cfaddca5 100644 --- a/rust/processor/src/transaction_filter.rs +++ b/rust/processor/src/transaction_filter.rs @@ -60,6 +60,8 @@ impl TransactionFilter { if let Some(focus_contract_addresses) = &self.focus_contract_addresses { // Skip if focus contract addresses are set and the transaction isn't in the list + // Question: Entry function transactions and Multisig transactions with entry function payload are treated + // the same way. Is this okay? if let Some(payload) = utr.payload.as_ref() { if let Some(Payload::EntryFunctionPayload(efp)) = payload.payload.as_ref() { if let Some(function) = efp.function.as_ref() { diff --git a/rust/processor/src/utils/util.rs b/rust/processor/src/utils/util.rs index 6746d51a5..346c05bdf 100644 --- a/rust/processor/src/utils/util.rs +++ b/rust/processor/src/utils/util.rs @@ -7,10 +7,7 @@ use crate::{ }; use aptos_protos::{ transaction::v1::{ - multisig_transaction_payload::Payload as MultisigPayloadType, - transaction_payload::Payload as PayloadType, write_set::WriteSet as WriteSetType, - EntryFunctionId, EntryFunctionPayload, MoveScriptBytecode, MoveType, ScriptPayload, - TransactionPayload, UserTransactionRequest, WriteSet, + multisig_transaction_payload::Payload as MultisigPayloadType, transaction_payload::{ExtraConfig, Payload as PayloadType}, write_set::WriteSet as WriteSetType, EntryFunctionId, EntryFunctionPayload, ExtraConfigV1, MoveScriptBytecode, MoveType, ScriptPayload, TransactionPayload, UserTransactionRequest, WriteSet }, util::timestamp::Timestamp, }; @@ -181,8 +178,19 @@ pub fn get_entry_function_function_name_from_user_request( }) } -pub fn get_payload_type(payload: &TransactionPayload) -> String { - payload.r#type().as_str_name().to_string() +// Question: Should we convert entry function payload to multisig payload here if the mutlisig_address is set in extra config? +pub fn get_payload_type(payload: &TransactionPayload) -> String { + let multisig_address = payload.extra_config.as_ref().and_then(|extra_config| match extra_config { + ExtraConfig::ExtraConfigV1(ExtraConfigV1 { + multisig_address, .. + }) => multisig_address.clone() + }); + let payload_type = payload.r#type().as_str_name().to_string(); + if multisig_address.is_some() && payload.r#type().as_str_name().to_string() == "TYPE_ENTRY_FUNCTION_PAYLOAD" { + "TYPE_MULTISIG_PAYLOAD".to_string() + } else { + payload_type + } } /// Part of the json comes escaped from the protobuf so we need to unescape in a safe way @@ -199,15 +207,44 @@ pub fn get_clean_payload(payload: &TransactionPayload, version: i64) -> Option multisig_address.clone() + }); match payload.payload.as_ref().unwrap() { PayloadType::EntryFunctionPayload(inner) => { - let clean = get_clean_entry_function_payload(inner, version); - Some(serde_json::to_value(clean).unwrap_or_else(|_| { - tracing::error!(version = version, "Unable to serialize payload into value"); - panic!() - })) + if let Some(multisig_address) = multisig_address { + let payload_clean = get_clean_entry_function_payload(inner, version); + let clean = MultisigPayloadClean { + multisig_address: multisig_address.clone(), + transaction_payload: Some(serde_json::to_value(payload_clean).unwrap_or_else(|_| { + tracing::error!( + version = version, + "Unable to serialize payload into value" + ); + panic!() + })), + }; + Some(serde_json::to_value(clean).unwrap_or_else(|_| { + tracing::error!(version = version, "Unable to serialize payload into value"); + panic!() + })) + } else { + let clean = get_clean_entry_function_payload(inner, version); + Some(serde_json::to_value(clean).unwrap_or_else(|_| { + tracing::error!(version = version, "Unable to serialize payload into value"); + panic!() + })) + } }, PayloadType::ScriptPayload(inner) => { + // TODO: We are assuming that multisig transactions do not use script payload. + // If that changes in the future, update this. + if multisig_address.is_some() { + tracing::error!(version = version, "Multisig address cannot be set for script payloads"); + } let clean = get_clean_script_payload(inner, version); Some(serde_json::to_value(clean).unwrap_or_else(|_| { tracing::error!(version = version, "Unable to serialize payload into value"); diff --git a/rust/sdk-processor/src/db/common/models/events_models/events.rs b/rust/sdk-processor/src/db/common/models/events_models/events.rs index fa7699d50..2db617311 100644 --- a/rust/sdk-processor/src/db/common/models/events_models/events.rs +++ b/rust/sdk-processor/src/db/common/models/events_models/events.rs @@ -21,6 +21,7 @@ const EVENT_TYPE_MAX_LENGTH: usize = 300; #[diesel(primary_key(transaction_version, event_index))] #[diesel(table_name = events)] pub struct Event { + // Question: Do we need to add a replay_protection_nonce here? pub sequence_number: i64, pub creation_number: i64, pub account_address: String, diff --git a/rust/testing-transactions/src/lib.rs b/rust/testing-transactions/src/lib.rs index d2782bd8e..a2706bbde 100644 --- a/rust/testing-transactions/src/lib.rs +++ b/rust/testing-transactions/src/lib.rs @@ -3,6 +3,8 @@ include!(concat!(env!("OUT_DIR"), "/generate_transactions.rs")); +// TODO: Add some orderless transactions here for testing. +// Question: Should we do it after the orderless tranactions feature is deployed? #[cfg(test)] mod tests { use super::*;