Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion hasura-api/metadata-json/unified.json
Original file line number Diff line number Diff line change
Expand Up @@ -2335,7 +2335,8 @@
"sender",
"sequence_number",
"timestamp",
"version"
"version",
"replay_protection_nonce"
Copy link
Contributor

Choose a reason for hiding this comment

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

we have a process to update this. https://app.clickup.com/9011498509/v/dc/8cj13gd-14211/8cj13gd-8791

our oncall might be able to help here though.

],
"filter": {},
"limit": 100
Expand Down
3 changes: 2 additions & 1 deletion hasura-api/metadata-json/unified_transition.json
Original file line number Diff line number Diff line change
Expand Up @@ -2384,7 +2384,8 @@
"sender",
"sequence_number",
"timestamp",
"version"
"version",
"replay_protection_nonce"
],
"filter": {},
"limit": 100
Expand Down
3 changes: 2 additions & 1 deletion python/utils/transaction_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
18 changes: 15 additions & 3 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
2 changes: 2 additions & 0 deletions rust/integration-tests/src/models/user_transactions_models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,6 @@ pub struct UserTransaction {
pub entry_function_contract_address: Option<String>,
pub entry_function_module_name: Option<String>,
pub entry_function_function_name: Option<String>,
// 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<i64>,
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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<u64>,
}

impl NamedTable for UserTransaction {
Expand Down Expand Up @@ -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()
})
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link
Contributor

Choose a reason for hiding this comment

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

up to you. it's useful for users but if you want it for debugging we could add it.

#[derive(Clone, Debug, Deserialize, FieldCount, Identifiable, Insertable, Serialize)]
#[diesel(primary_key(transaction_version, event_index))]
#[diesel(table_name = events)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link
Contributor

Choose a reason for hiding this comment

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

no b/c it's not needed for this table anyways.

pub event_sequence_number: i64,
pub token_data_id_hash: String,
pub property_version: BigDecimal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -43,6 +43,8 @@ pub struct UserTransaction {
pub entry_function_contract_address: Option<String>,
pub entry_function_module_name: Option<String>,
pub entry_function_function_name: Option<String>,
// Question: Is it okay to use i64 here, as the nonce is defined as u64 in aptos-core repo?
pub replay_protection_nonce: Option<i64>,
}

impl UserTransaction {
Expand Down Expand Up @@ -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),
)
Expand Down
1 change: 1 addition & 0 deletions rust/processor/src/db/postgres/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1271,6 +1271,7 @@ diesel::table! {
entry_function_module_name -> Nullable<Varchar>,
#[max_length = 255]
entry_function_function_name -> Nullable<Varchar>,
replay_protection_nonce -> Nullable<Int8>,
}
}

Expand Down
2 changes: 2 additions & 0 deletions rust/processor/src/transaction_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
59 changes: 48 additions & 11 deletions rust/processor/src/utils/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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
Expand All @@ -199,15 +207,44 @@ pub fn get_clean_payload(payload: &TransactionPayload, version: i64) -> Option<V
);
return None;
}
// Question: Should we convert entry function payload to multisig payload here if the mutlisig_address is set in extra config?
let multisig_address = payload.extra_config.as_ref().and_then(|extra_config| match extra_config {
ExtraConfig::ExtraConfigV1(ExtraConfigV1 {
multisig_address, ..
}) => 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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions rust/testing-transactions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down
Loading