Skip to content

Commit

Permalink
[rpc] Make tx cert optional
Browse files Browse the repository at this point in the history
  • Loading branch information
lxfind committed Feb 2, 2023
1 parent a570c28 commit 7fe3d27
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 34 deletions.
2 changes: 1 addition & 1 deletion apps/explorer/tests/search.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ test('can search for transaction', async ({ page }) => {
const address = await faucet();
const tx = await mint(address);

const txid = tx.certificate.transactionDigest;
const txid = tx.effects.effects.transactionDigest;
await page.goto('/');
await search(page, txid);
await expect(page).toHaveURL(`/transaction/${txid}`);
Expand Down
4 changes: 2 additions & 2 deletions apps/explorer/tests/transaction.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { faucet, mint } from './utils/localnet';
test('displays the transaction timestamp', async ({ page }) => {
const address = await faucet();
const tx = await mint(address);
const txid = tx.certificate.transactionDigest;
const txid = tx.effects.effects.transactionDigest;
await page.goto(`/transaction/${txid}`);
await expect(
page.getByTestId('transaction-timestamp').locator('div').nth(1)
Expand All @@ -17,7 +17,7 @@ test('displays the transaction timestamp', async ({ page }) => {
test('displays gas breakdown', async ({ page }) => {
const address = await faucet();
const tx = await mint(address);
const txid = tx.certificate.transactionDigest;
const txid = tx.effects.effects.transactionDigest;
await page.goto(`/transaction/${txid}`);
await expect(page.getByTestId('gas-breakdown')).toBeVisible();
});
6 changes: 3 additions & 3 deletions crates/sui-core/src/transaction_orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ where
} = response;
if !wait_for_local_execution {
return Ok(ExecuteTransactionResponse::EffectsCert(Box::new((
tx_cert.into(),
Some(tx_cert.into()),
effects_cert.into(),
false,
))));
Expand All @@ -206,12 +206,12 @@ where
.await
{
Ok(_) => Ok(ExecuteTransactionResponse::EffectsCert(Box::new((
tx_cert.into(),
Some(tx_cert.into()),
effects_cert.into(),
true,
)))),
Err(_) => Ok(ExecuteTransactionResponse::EffectsCert(Box::new((
tx_cert.into(),
Some(tx_cert.into()),
effects_cert.into(),
false,
)))),
Expand Down
9 changes: 7 additions & 2 deletions crates/sui-json-rpc-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,9 @@ pub struct SuiTBlsSignRandomnessObjectResponse {
#[allow(clippy::large_enum_variant)]
#[derive(Serialize, Deserialize, Debug, JsonSchema)]
pub struct SuiExecuteTransactionResponse {
pub certificate: SuiCertifiedTransaction,
// If this transaction was already finalized previously, there is no guarantee that a
// certificate is still available.
pub certificate: Option<SuiCertifiedTransaction>,
pub effects: SuiCertifiedTransactionEffects,
// If the transaction is confirmed to be executed locally
// before this response.
Expand All @@ -428,7 +430,10 @@ impl SuiExecuteTransactionResponse {
Ok(match resp {
ExecuteTransactionResponse::EffectsCert(cert) => {
let (certificate, effects, is_executed_locally) = *cert;
let certificate: SuiCertifiedTransaction = certificate.try_into()?;
let certificate: Option<SuiCertifiedTransaction> = match certificate {
Some(c) => Some(c.try_into()?),
None => None,
};
let effects: SuiCertifiedTransactionEffects =
SuiCertifiedTransactionEffects::try_from(effects, resolver)?;
SuiExecuteTransactionResponse {
Expand Down
10 changes: 8 additions & 2 deletions crates/sui-open-rpc/spec/openrpc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5298,13 +5298,19 @@
"SuiExecuteTransactionResponse": {
"type": "object",
"required": [
"certificate",
"confirmed_local_execution",
"effects"
],
"properties": {
"certificate": {
"$ref": "#/components/schemas/CertifiedTransaction"
"anyOf": [
{
"$ref": "#/components/schemas/CertifiedTransaction"
},
{
"type": "null"
}
]
},
"confirmed_local_execution": {
"type": "boolean"
Expand Down
12 changes: 6 additions & 6 deletions crates/sui-sdk/src/apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ impl QuorumDriver {
tx: VerifiedTransaction,
request_type: Option<ExecuteTransactionRequestType>,
) -> SuiRpcResult<TransactionExecutionResult> {
let tx_digest = *tx.digest();
let (tx_bytes, signature) = tx.to_tx_bytes_and_signature();
let request_type =
request_type.unwrap_or(ExecuteTransactionRequestType::WaitForLocalExecution);
Expand All @@ -495,21 +496,20 @@ impl QuorumDriver {

Ok(match request_type {
ExecuteTransactionRequestType::WaitForEffectsCert => TransactionExecutionResult {
tx_digest: certificate.transaction_digest,
tx_cert: Some(certificate),
tx_digest,
tx_cert: certificate,
effects: Some(effects.effects),
confirmed_local_execution,
timestamp_ms: None,
parsed_data: None,
},
ExecuteTransactionRequestType::WaitForLocalExecution => {
if !confirmed_local_execution {
Self::wait_until_fullnode_sees_tx(&self.api, certificate.transaction_digest)
.await?;
Self::wait_until_fullnode_sees_tx(&self.api, tx_digest).await?;
}
TransactionExecutionResult {
tx_digest: certificate.transaction_digest,
tx_cert: Some(certificate),
tx_digest,
tx_cert: certificate,
effects: Some(effects.effects),
confirmed_local_execution,
timestamp_ms: None,
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-types/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2427,7 +2427,7 @@ pub type IsTransactionExecutedLocally = bool;
pub enum ExecuteTransactionResponse {
EffectsCert(
Box<(
CertifiedTransaction,
Option<CertifiedTransaction>,
CertifiedTransactionEffects,
IsTransactionExecutedLocally,
)>,
Expand Down
22 changes: 11 additions & 11 deletions crates/sui/tests/full_node_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ async fn test_full_node_transaction_orchestrator_basic() -> Result<(), anyhow::E
effects_cert: certified_txn_effects,
} = rx.recv().await.unwrap().unwrap();
let (ct, cte, is_executed_locally) = *res;
assert_eq!(*ct.digest(), digest);
assert_eq!(*ct.unwrap().digest(), digest);
assert_eq!(*certified_txn.digest(), digest);
assert_eq!(*cte.digest(), *certified_txn_effects.digest());
assert!(is_executed_locally);
Expand All @@ -980,7 +980,7 @@ async fn test_full_node_transaction_orchestrator_basic() -> Result<(), anyhow::E
effects_cert: certified_txn_effects,
} = rx.recv().await.unwrap().unwrap();
let (ct, cte, is_executed_locally) = *res;
assert_eq!(*ct.digest(), digest);
assert_eq!(*ct.unwrap().digest(), digest);
assert_eq!(*certified_txn.digest(), digest);
assert_eq!(*cte.digest(), *certified_txn_effects.digest());
assert!(!is_executed_locally);
Expand Down Expand Up @@ -1037,11 +1037,11 @@ async fn test_execute_tx_with_serialized_signature() -> Result<(), anyhow::Error
.unwrap();

let SuiExecuteTransactionResponse {
certificate,
effects: _,
certificate: _,
effects,
confirmed_local_execution,
} = response;
assert_eq!(&certificate.transaction_digest, tx_digest);
assert_eq!(&effects.effects.transaction_digest, tx_digest);
assert!(confirmed_local_execution);
}
Ok(())
Expand Down Expand Up @@ -1077,11 +1077,11 @@ async fn test_full_node_transaction_orchestrator_rpc_ok() -> Result<(), anyhow::
.unwrap();

let SuiExecuteTransactionResponse {
certificate,
effects: _,
certificate: _,
effects,
confirmed_local_execution,
} = response;
assert_eq!(&certificate.transaction_digest, tx_digest);
assert_eq!(&effects.effects.transaction_digest, tx_digest);
assert!(confirmed_local_execution);

let _response: SuiTransactionResponse = jsonrpc_client
Expand All @@ -1102,11 +1102,11 @@ async fn test_full_node_transaction_orchestrator_rpc_ok() -> Result<(), anyhow::
.unwrap();

let SuiExecuteTransactionResponse {
certificate,
effects: _,
certificate: _,
effects,
confirmed_local_execution,
} = response;
assert_eq!(&certificate.transaction_digest, tx_digest);
assert_eq!(&effects.effects.transaction_digest, tx_digest);
assert!(!confirmed_local_execution);

Ok(())
Expand Down
10 changes: 5 additions & 5 deletions crates/sui/tests/transaction_orchestrator_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use sui_core::transaction_orchestrator::TransactiondOrchestrator;
use sui_macros::sim_test;
use sui_types::crypto::{get_key_pair, AccountKeyPair};
use sui_types::messages::{
CertifiedTransaction, ExecuteTransactionRequest, ExecuteTransactionRequestType,
CertifiedTransactionEffects, ExecuteTransactionRequest, ExecuteTransactionRequestType,
ExecuteTransactionResponse, TransactionData, VerifiedTransaction,
};
use sui_types::object::generate_test_gas_objects_with_owner;
Expand Down Expand Up @@ -206,7 +206,7 @@ async fn test_tx_across_epoch_boundaries() {
let (sender, keypair) = get_key_pair::<AccountKeyPair>();
let gas_objects = generate_test_gas_objects_with_owner(1, sender);
let (result_tx, mut result_rx) =
tokio::sync::mpsc::channel::<CertifiedTransaction>(total_tx_cnt);
tokio::sync::mpsc::channel::<CertifiedTransactionEffects>(total_tx_cnt);

let (config, mut gas_objects) = test_authority_configs_with_objects(gas_objects);
let authorities = spawn_test_authorities([], &config).await;
Expand Down Expand Up @@ -247,8 +247,8 @@ async fn test_tx_across_epoch_boundaries() {
{
Ok(ExecuteTransactionResponse::EffectsCert(res)) => {
info!(?tx_digest, "tx result: ok");
let (tx_cert, _, _) = *res;
result_tx.send(tx_cert).await.unwrap();
let (_, effects_cert, _) = *res;
result_tx.send(effects_cert).await.unwrap();
}
Err(QuorumDriverError::TimeoutBeforeFinality) => {
info!(?tx_digest, "tx result: timeout and will retry")
Expand Down Expand Up @@ -277,7 +277,7 @@ async fn test_tx_across_epoch_boundaries() {
// The transaction must finalize in epoch 1
let start = std::time::Instant::now();
match tokio::time::timeout(tokio::time::Duration::from_secs(15), result_rx.recv()).await {
Ok(Some(tx_cert)) if tx_cert.auth_sig().epoch == 1 => (),
Ok(Some(effects_cert)) if effects_cert.epoch() == 1 => (),
other => panic!("unexpected error: {:?}", other),
}
info!("test completed in {:?}", start.elapsed());
Expand Down
2 changes: 1 addition & 1 deletion sdk/typescript/src/types/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export const SuiExecuteTransactionResponse = union([
}),
}),
object({
certificate: CertifiedTransaction,
certificate: optional(CertifiedTransaction),
effects: SuiCertifiedTransactionEffects,
confirmed_local_execution: boolean(),
}),
Expand Down

0 comments on commit 7fe3d27

Please sign in to comment.