Skip to content

Commit

Permalink
[core] Refactor Transaction to use MessageEnvelope (MystenLabs#5915)
Browse files Browse the repository at this point in the history
  • Loading branch information
lxfind authored Nov 8, 2022
1 parent 17c6c72 commit f0ade29
Show file tree
Hide file tree
Showing 43 changed files with 400 additions and 680 deletions.
8 changes: 5 additions & 3 deletions crates/sui-cluster-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ impl TestContext {
.get_fullnode_client()
.quorum_driver()
.execute_transaction(
Transaction::new(txn_data, signature).verify().unwrap(),
Transaction::from_data(txn_data, signature)
.verify()
.unwrap(),
Some(ExecuteTransactionRequestType::WaitForLocalExecution),
)
.await
Expand Down Expand Up @@ -286,8 +288,8 @@ impl ClusterTest {
let mut success_cnt = 0;
let total_cnt = tests.len() as i32;
for t in tests {
let is_sucess = t.run(&mut ctx).await as i32;
success_cnt += is_sucess;
let is_success = t.run(&mut ctx).await as i32;
success_cnt += is_success;
}
if success_cnt < total_cnt {
// If any test failed, panic to bubble up the signal
Expand Down
21 changes: 10 additions & 11 deletions crates/sui-core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ impl AuthorityState {

let (_gas_status, input_objects) = transaction_input_checker::check_transaction_input(
&self.database,
&transaction.signed_data.data,
&transaction.data().data,
)
.await?;

Expand All @@ -601,7 +601,7 @@ impl AuthorityState {
transaction: VerifiedTransaction,
) -> Result<VerifiedTransactionInfoResponse, SuiError> {
let transaction_digest = *transaction.digest();
debug!(tx_digest=?transaction_digest, "handle_transaction. Tx data: {:?}", transaction.signed_data.data);
debug!(tx_digest=?transaction_digest, "handle_transaction. Tx data: {:?}", transaction.data().data);
let _metrics_guard = start_timer(self.metrics.handle_transaction_latency.clone());

self.metrics.tx_orders.inc();
Expand Down Expand Up @@ -675,7 +675,7 @@ impl AuthorityState {
?observed_effects_digest,
?effects.effects,
?resp.signed_effects,
input_objects = ?certificate.signed_data.data.input_objects(),
input_objects = ?certificate.data().data.input_objects(),
"Locally executed effects do not match canonical effects!");
}
Ok(())
Expand Down Expand Up @@ -732,7 +732,7 @@ impl AuthorityState {
let span = tracing::debug_span!(
"validator_acquire_tx_guard",
?tx_digest,
tx_kind = certificate.signed_data.data.kind_as_str()
tx_kind = certificate.data().data.kind_as_str()
);
let tx_guard = self
.database
Expand Down Expand Up @@ -941,7 +941,7 @@ impl AuthorityState {
.observe(shared_object_count as f64);
self.metrics
.batch_size
.observe(certificate.signed_data.data.kind.batch_size() as f64);
.observe(certificate.data().data.kind.batch_size() as f64);

Ok(VerifiedTransactionInfoResponse {
signed_transaction: self.database.get_transaction(&digest)?,
Expand Down Expand Up @@ -972,8 +972,7 @@ impl AuthorityState {
// At this point we need to check if any shared objects need locks,
// and whether they have them.
let shared_object_refs = input_objects.filter_shared_objects();
if !shared_object_refs.is_empty() && !certificate.signed_data.data.kind.is_change_epoch_tx()
{
if !shared_object_refs.is_empty() && !certificate.data().data.kind.is_change_epoch_tx() {
// If the transaction contains shared objects, we need to ensure they have been scheduled
// for processing by the consensus protocol.
// There is no need to go through consensus for system transactions that can
Expand All @@ -996,7 +995,7 @@ impl AuthorityState {
execution_engine::execute_transaction_to_effects(
shared_object_refs,
temporary_store,
certificate.signed_data.data.clone(),
certificate.data().data.clone(),
transaction_digest,
transaction_dependencies,
&self.move_vm,
Expand Down Expand Up @@ -1066,7 +1065,7 @@ impl AuthorityState {
) -> SuiResult {
indexes.index_tx(
cert.sender_address(),
cert.signed_data
cert.data()
.data
.input_objects()?
.iter()
Expand All @@ -1075,7 +1074,7 @@ impl AuthorityState {
.effects
.all_mutated()
.map(|(obj_ref, owner, _kind)| (*obj_ref, *owner)),
cert.signed_data
cert.data()
.data
.move_calls()
.iter()
Expand Down Expand Up @@ -2190,7 +2189,7 @@ impl AuthorityState {
fn verify_narwhal_transaction(&self, certificate: &CertifiedTransaction) -> SuiResult {
// Check the certificate. Remember that Byzantine authorities may input anything into
// consensus.
certificate.verify_signatures(&self.committee.load())
certificate.verify_signature(&self.committee.load())
}

/// Verifies transaction signatures and other data
Expand Down
11 changes: 6 additions & 5 deletions crates/sui-core/src/authority/authority_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use sui_storage::{
};
use sui_types::batch::TxSequenceNumber;
use sui_types::crypto::{AuthoritySignInfo, EmptySignInfo};
use sui_types::message_envelope::VerifiedEnvelope;
use sui_types::object::Owner;
use sui_types::storage::{ChildObjectResolver, SingleTxContext, WriteKind};
use sui_types::{base_types::SequenceNumber, storage::ParentSync};
Expand Down Expand Up @@ -405,7 +406,7 @@ impl<S: Eq + Debug + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
pub async fn get_object_locking_transaction(
&self,
object_ref: &ObjectRef,
) -> SuiResult<Option<VerifiedTransactionEnvelope<S>>> {
) -> SuiResult<Option<VerifiedEnvelope<SenderSignedData, S>>> {
let tx_lock = self.lock_service.get_lock(*object_ref).await?.ok_or(
SuiError::ObjectLockUninitialized {
obj_ref: *object_ref,
Expand Down Expand Up @@ -614,7 +615,7 @@ impl<S: Eq + Debug + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
&self,
epoch: EpochId,
owned_input_objects: &[ObjectRef],
transaction: VerifiedTransactionEnvelope<S>,
transaction: VerifiedEnvelope<SenderSignedData, S>,
) -> Result<(), SuiError> {
let tx_digest = *transaction.digest();

Expand Down Expand Up @@ -1329,7 +1330,7 @@ impl<S: Eq + Debug + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
pub fn get_transaction(
&self,
transaction_digest: &TransactionDigest,
) -> SuiResult<Option<VerifiedTransactionEnvelope<S>>> {
) -> SuiResult<Option<VerifiedEnvelope<SenderSignedData, S>>> {
let transaction = self.epoch_tables().transactions.get(transaction_digest)?;
Ok(transaction.map(|t| t.into()))
}
Expand Down Expand Up @@ -1370,13 +1371,13 @@ impl SuiDataStore<AuthoritySignInfo> {
cur_epoch: EpochId,
transaction_digest: &TransactionDigest,
) -> SuiResult<bool> {
let tx: Option<VerifiedTransactionEnvelope<AuthoritySignInfo>> = self
let tx: Option<VerifiedSignedTransaction> = self
.epoch_tables()
.transactions
.get(transaction_digest)?
.map(|t| t.into());
Ok(if let Some(signed_tx) = tx {
signed_tx.auth_sign_info.epoch == cur_epoch
signed_tx.auth_sig().epoch == cur_epoch
} else {
false
})
Expand Down
5 changes: 3 additions & 2 deletions crates/sui-core/src/authority/authority_store_tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,19 @@ use std::path::Path;
use sui_storage::default_db_options;
use sui_types::base_types::{ExecutionDigests, SequenceNumber};
use sui_types::batch::{SignedBatch, TxSequenceNumber};
use sui_types::messages::{TrustedCertificate, TrustedTransactionEnvelope};
use sui_types::messages::TrustedCertificate;
use typed_store::rocks::{DBMap, DBOptions};
use typed_store::traits::TypedStoreDebug;

use sui_types::message_envelope::TrustedEnvelope;
use typed_store_derive::DBMapUtils;

/// AuthorityEpochTables contains tables that contain data that is only valid within an epoch.
#[derive(DBMapUtils)]
pub struct AuthorityEpochTables<S> {
/// This is map between the transaction digest and transactions found in the `transaction_lock`.
#[default_options_override_fn = "transactions_table_default_config"]
pub(crate) transactions: DBMap<TransactionDigest, TrustedTransactionEnvelope<S>>,
pub(crate) transactions: DBMap<TransactionDigest, TrustedEnvelope<SenderSignedData, S>>,

/// The pending execution table holds a sequence of transactions that are present
/// in the certificates table, but may not have yet been executed, and should be executed.
Expand Down
16 changes: 8 additions & 8 deletions crates/sui-core/src/authority_aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ where
// Extract the set of authorities that should have this certificate
// and its full history. We should be able to use these are source authorities.
let mut candidate_source_authorties: HashSet<AuthorityName> = cert
.auth_sign_info
.auth_sig()
.authorities(committee)
.collect::<SuiResult<HashSet<_>>>()?
.iter()
Expand Down Expand Up @@ -1604,7 +1604,7 @@ where
validity_threshold = validity,
"Broadcasting transaction request to authorities"
);
trace!("Transaction data: {:?}", transaction.signed_data.data);
trace!("Transaction data: {:?}", transaction.data().data);

#[derive(Default)]
struct ProcessTransactionState {
Expand Down Expand Up @@ -1677,10 +1677,10 @@ where
Ok(VerifiedTransactionInfoResponse {
signed_transaction: Some(inner_signed_transaction),
..
}) if inner_signed_transaction.auth_sign_info.epoch == self.committee.epoch => {
}) if inner_signed_transaction.auth_sig().epoch == self.committee.epoch => {
let tx_digest = inner_signed_transaction.digest();
debug!(tx_digest = ?tx_digest, ?name, weight, "Received signed transaction from validator handle_transaction");
state.signatures.push(inner_signed_transaction.into_inner().auth_sign_info);
state.signatures.push(inner_signed_transaction.into_inner().into_data_and_sig().1);
state.good_stake += weight;
if state.good_stake >= threshold {
self.metrics
Expand All @@ -1689,8 +1689,8 @@ where
self.metrics.num_good_stake.observe(state.good_stake as f64);
self.metrics.num_bad_stake.observe(state.bad_stake as f64);
state.certificate =
Some(CertifiedTransaction::new_with_auth_sign_infos(
transaction_ref.clone(),
Some( CertifiedTransaction::new(
transaction_ref.data().clone(),
state.signatures.clone(),
&self.committee,
)?.verify(&self.committee)?);
Expand Down Expand Up @@ -1741,7 +1741,7 @@ where
?tx_digest,
name=?name.concise(),
expected_epoch=?self.committee.epoch,
returned_epoch=?inner_signed.auth_sign_info.epoch,
returned_epoch=?inner_signed.auth_sig().epoch,
"Returned signed transaction is from wrong epoch"
);
}
Expand Down Expand Up @@ -2291,7 +2291,7 @@ where
}

let signers: BTreeSet<_> = cert
.auth_sign_info
.auth_sig()
.authorities(&self.committee)
.filter_map(|r| r.ok())
.cloned()
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-core/src/authority_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ impl ValidatorService {
let span = tracing::debug_span!(
"validator_state_process_tx",
?tx_digest,
tx_kind = transaction.signed_data.data.kind_as_str()
tx_kind = transaction.data().data.kind_as_str()
);

let info = state
Expand Down Expand Up @@ -409,7 +409,7 @@ impl ValidatorService {

// 3) If the validator is already halted, we stop here, to avoid
// sending the transaction to consensus.
if state.is_halted() && !certificate.signed_data.data.kind.is_system_tx() {
if state.is_halted() && !certificate.data().data.kind.is_system_tx() {
return Err(tonic::Status::from(SuiError::ValidatorHaltedAtEpochEnd));
}

Expand Down Expand Up @@ -442,7 +442,7 @@ impl ValidatorService {
let span = tracing::debug_span!(
"validator_state_process_cert",
?tx_digest,
tx_kind = certificate.signed_data.data.kind_as_str()
tx_kind = certificate.data().data.kind_as_str()
);
match state
.handle_certificate(&certificate)
Expand Down
6 changes: 2 additions & 4 deletions crates/sui-core/src/checkpoints/tests/checkpoint_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1181,14 +1181,12 @@ async fn set_fragment_external() {
for key_pair in keys.iter() {
sigs.push(AuthoritySignInfo::new(
0,
&tx.signed_data,
tx.data(),
key_pair.public().into(),
key_pair,
));
}
let mut certificate =
CertifiedTransaction::new_with_auth_sign_infos(tx, sigs, &committee).unwrap();
certificate.auth_sign_info.epoch = committee.epoch();
let certificate = CertifiedTransaction::new(tx.into_message(), sigs, &committee).unwrap();
fragment12.data.certs.insert(digest, certificate);
}
// let fragment13 = p1.diff_with(&p3);
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-core/src/epoch/reconfiguration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ where
let err = match self
.net
.load()
.process_transaction(advance_epoch_tx.clone().to_transaction())
.process_transaction(advance_epoch_tx.clone().into_unsigned())
.await
{
Ok(certificate) => match self.state.handle_certificate(&certificate).await {
Expand Down
19 changes: 7 additions & 12 deletions crates/sui-core/src/epoch/tests/reconfiguration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,13 @@ async fn test_start_epoch_change() {
// Test that when validator is halted, we cannot send any certificate.
let sigs = states
.iter()
.map(|state| {
AuthoritySignInfo::new(0, &transaction.signed_data, state.name, &*state.secret)
})
.map(|state| AuthoritySignInfo::new(0, transaction.data(), state.name, &*state.secret))
.collect();
let certificate = CertifiedTransaction::new_with_auth_sign_infos(
transaction.clone(),
sigs,
&genesis_committee,
)
.unwrap()
.verify(&genesis_committee)
.unwrap();
let certificate =
CertifiedTransaction::new(transaction.clone().into_message(), sigs, &genesis_committee)
.unwrap()
.verify(&genesis_committee)
.unwrap();
assert_eq!(
state.handle_certificate(&certificate).await.unwrap_err(),
SuiError::ValidatorHaltedAtEpochEnd
Expand Down Expand Up @@ -347,7 +342,7 @@ async fn test_cross_epoch_effects_cert() {
net.committee.epoch += 1;
// Call to execute_transaction can still succeed.
let (tx_cert, effects_cert) = net.execute_transaction(&transaction).await.unwrap();
assert_eq!(tx_cert.auth_sign_info.epoch, 0);
assert_eq!(tx_cert.auth_sig().epoch, 0);
assert_eq!(effects_cert.auth_signature.epoch, 1);
}

Expand Down
Loading

0 comments on commit f0ade29

Please sign in to comment.