Skip to content

Commit

Permalink
Expose GasData through executor (#21244)
Browse files Browse the repository at this point in the history
## Description 
Changed executor API to take `GasData` instead of gas payments only
(`Vec<ObjectRef>`). That allows us to expose more info about gas and
sponsor to the `TxContext` in execution.
I wondered whether the info about sponsor and more (e.g. gas price and
budget) should be computed on the client of the executor (e.g. in
`authority.rs` and I had the code initially that way) but it makes for a
more intrusive change.
Passing the `GasData` allows the executor to compute everything is
needed for execution with a more stable API. Thoughts welcome.

This PR should change nothing in the current implementation and though
`sponsor` and `gas_price` are exposed to the `TxContext` (the Rust side)
they are unused at the moment (in this PR, later PRs will use that).

`authority::dev_inspect_transaction_block` has changed a bit to make it
more readable for me. I believe it is correct but please @emmazzz @amnn
@jordanjennings-mysten take a look, I may have missed something.

The replay data structure is changed to add the sponsor, however we are
not changing the serialization at the moment and old serialized blobs
should still work (thanks @tzakian for the suggestion).


## Test plan 

Existing tests as this changes nothing

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] gRPC:
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
  • Loading branch information
dariorussi authored Feb 20, 2025
1 parent 9623f84 commit 447b0a3
Show file tree
Hide file tree
Showing 21 changed files with 182 additions and 92 deletions.
4 changes: 2 additions & 2 deletions crates/simulacrum/src/epoch_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl EpochState {
)?;

let transaction_data = transaction.data().transaction_data();
let (kind, signer, gas) = transaction_data.execution_parts();
let (kind, signer, gas_data) = transaction_data.execution_parts();
let (inner_temp_store, gas_status, effects, _timings, result) =
self.executor.execute_transaction_to_effects(
store.backing_store(),
Expand All @@ -145,7 +145,7 @@ impl EpochState {
&self.epoch_start_state.epoch(),
self.epoch_start_state.epoch_start_timestamp_ms(),
checked_input_objects,
gas,
gas_data,
gas_status,
kind,
signer,
Expand Down
50 changes: 26 additions & 24 deletions crates/sui-core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1664,7 +1664,7 @@ impl AuthorityState {
let tx_digest = *certificate.digest();
let protocol_config = epoch_store.protocol_config();
let transaction_data = &certificate.data().intent_message().value;
let (kind, signer, gas) = transaction_data.execution_parts();
let (kind, signer, gas_data) = transaction_data.execution_parts();

#[allow(unused_mut)]
let (inner_temp_store, _, mut effects, timings, execution_error_opt) =
Expand All @@ -1684,7 +1684,7 @@ impl AuthorityState {
.epoch_data()
.epoch_start_timestamp(),
input_objects,
gas,
gas_data,
gas_status,
kind,
signer,
Expand Down Expand Up @@ -1813,7 +1813,7 @@ impl AuthorityState {
)?;

// make a gas object if one was not provided
let mut gas_object_refs = transaction.gas().to_vec();
let mut gas_data = transaction.gas_data().clone();
let ((gas_status, checked_input_objects), mock_gas) = if transaction.gas().is_empty() {
let sender = transaction.sender();
// use a 1B sui coin
Expand All @@ -1827,7 +1827,7 @@ impl AuthorityState {
TransactionDigest::genesis_marker(),
);
let gas_object_ref = gas_object.compute_object_reference();
gas_object_refs = vec![gas_object_ref];
gas_data.payment = vec![gas_object_ref];
(
sui_transaction_checks::check_transaction_input_with_given_gas(
epoch_store.protocol_config(),
Expand Down Expand Up @@ -1877,7 +1877,7 @@ impl AuthorityState {
.epoch_data()
.epoch_start_timestamp(),
checked_input_objects,
gas_object_refs,
gas_data,
gas_status,
kind,
signer,
Expand Down Expand Up @@ -2004,7 +2004,7 @@ impl AuthorityState {
)?;

// make a gas object if one was not provided
let mut gas_object_refs = transaction.gas().to_vec();
let mut gas_data = transaction.gas_data().clone();
let ((gas_status, checked_input_objects), mock_gas) = if transaction.gas().is_empty() {
let sender = transaction.sender();
// use a 1B sui coin
Expand All @@ -2018,7 +2018,7 @@ impl AuthorityState {
TransactionDigest::genesis_marker(),
);
let gas_object_ref = gas_object.compute_object_reference();
gas_object_refs = vec![gas_object_ref];
gas_data.payment = vec![gas_object_ref];
(
sui_transaction_checks::check_transaction_input_with_given_gas(
epoch_store.protocol_config(),
Expand Down Expand Up @@ -2068,7 +2068,7 @@ impl AuthorityState {
.epoch_data()
.epoch_start_timestamp(),
checked_input_objects,
gas_object_refs,
gas_data,
gas_status,
kind,
signer,
Expand Down Expand Up @@ -2124,7 +2124,7 @@ impl AuthorityState {
let owner = gas_sponsor.unwrap_or(sender);
// Payment might be empty here, but it's fine we'll have to deal with it later after reading all the input objects.
let payment = gas_objects.unwrap_or_default();
let transaction = TransactionData::V1(TransactionDataV1 {
let mut transaction = TransactionData::V1(TransactionDataV1 {
kind: transaction_kind.clone(),
sender,
gas_data: GasData {
Expand Down Expand Up @@ -2170,26 +2170,20 @@ impl AuthorityState {
.unwrap_or(false),
)?;

// Create and use a dummy gas object if there is no gas object provided.
let dummy_gas_object = Object::new_gas_with_balance_and_owner_for_testing(
DEV_INSPECT_GAS_COIN_VALUE,
transaction.gas_owner(),
);

let gas_objects = if transaction.gas().is_empty() {
let gas_object_ref = dummy_gas_object.compute_object_reference();
vec![gas_object_ref]
} else {
transaction.gas().to_vec()
};

let (gas_status, checked_input_objects) = if skip_checks {
// If we are skipping checks, then we call the check_dev_inspect_input function which will perform
// only lightweight checks on the transaction input. And if the gas field is empty, that means we will
// use the dummy gas object so we need to add it to the input objects vector.
if transaction.gas().is_empty() {
// Create and use a dummy gas object if there is no gas object provided.
let dummy_gas_object = Object::new_gas_with_balance_and_owner_for_testing(
DEV_INSPECT_GAS_COIN_VALUE,
transaction.gas_owner(),
);
let gas_object_ref = dummy_gas_object.compute_object_reference();
transaction.gas_data_mut().payment = vec![gas_object_ref];
input_objects.push(ObjectReadResult::new(
InputObjectKind::ImmOrOwnedMoveObject(gas_objects[0]),
InputObjectKind::ImmOrOwnedMoveObject(gas_object_ref),
dummy_gas_object.into(),
));
}
Expand All @@ -2211,6 +2205,13 @@ impl AuthorityState {
// If we are not skipping checks, then we call the check_transaction_input function and its dummy gas
// variant which will perform full fledged checks just like a real transaction execution.
if transaction.gas().is_empty() {
// Create and use a dummy gas object if there is no gas object provided.
let dummy_gas_object = Object::new_gas_with_balance_and_owner_for_testing(
DEV_INSPECT_GAS_COIN_VALUE,
transaction.gas_owner(),
);
let gas_object_ref = dummy_gas_object.compute_object_reference();
transaction.gas_data_mut().payment = vec![gas_object_ref];
sui_transaction_checks::check_transaction_input_with_given_gas(
epoch_store.protocol_config(),
epoch_store.reference_gas_price(),
Expand All @@ -2236,6 +2237,7 @@ impl AuthorityState {

let executor = sui_execution::executor(protocol_config, /* silent */ true, None)
.expect("Creating an executor should not fail here");
let gas_data = transaction.gas_data().clone();
let intent_msg = IntentMessage::new(
Intent {
version: IntentVersion::V0,
Expand All @@ -2257,7 +2259,7 @@ impl AuthorityState {
.epoch_data()
.epoch_start_timestamp(),
checked_input_objects,
gas_objects,
gas_data,
gas_status,
transaction_kind,
sender,
Expand Down
20 changes: 16 additions & 4 deletions crates/sui-core/src/unit_tests/authority_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1690,8 +1690,14 @@ async fn test_publish_dependent_module_ok() {
);
let transaction = to_sender_signed_transaction(data, &sender_key);

let dependent_module_id =
TxContext::new(&sender, transaction.digest(), &EpochData::new_test()).fresh_id();
let dependent_module_id = TxContext::new(
&sender,
transaction.digest(),
&EpochData::new_test(),
rgp,
None,
)
.fresh_id();

// Object does not exist
assert!(authority.get_object(&dependent_module_id).await.is_none());
Expand Down Expand Up @@ -1739,8 +1745,14 @@ async fn test_publish_module_no_dependencies_ok() {
rgp,
);
let transaction = to_sender_signed_transaction(data, &sender_key);
let _module_object_id =
TxContext::new(&sender, transaction.digest(), &EpochData::new_test()).fresh_id();
let _module_object_id = TxContext::new(
&sender,
transaction.digest(),
&EpochData::new_test(),
rgp,
None,
)
.fresh_id();
let signed_effects = send_and_confirm_transaction(&authority, transaction)
.await
.unwrap()
Expand Down
5 changes: 3 additions & 2 deletions crates/sui-genesis-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,8 @@ fn create_genesis_transaction(
let expensive_checks = false;
let certificate_deny_set = HashSet::new();
let transaction_data = &genesis_transaction.data().intent_message().value;
let (kind, signer, _) = transaction_data.execution_parts();
let (kind, signer, mut gas_data) = transaction_data.execution_parts();
gas_data.payment = vec![];
let input_objects = CheckedInputObjects::new_for_genesis(vec![]);
let (inner_temp_store, _, effects, _timings, _execution_error) = executor
.execute_transaction_to_effects(
Expand All @@ -926,7 +927,7 @@ fn create_genesis_transaction(
&epoch_data.epoch_id(),
epoch_data.epoch_start_timestamp(),
input_objects,
vec![],
gas_data,
SuiGasStatus::new_unmetered(),
kind,
signer,
Expand Down
31 changes: 23 additions & 8 deletions crates/sui-replay/src/replay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use sui_sdk::{SuiClient, SuiClientBuilder};
use sui_types::in_memory_storage::InMemoryStorage;
use sui_types::message_envelope::Message;
use sui_types::storage::{get_module, PackageObject};
use sui_types::transaction::GasData;
use sui_types::transaction::TransactionKind::ProgrammableTransaction;
use sui_types::SUI_DENY_LIST_OBJECT_ID;
use sui_types::{
Expand Down Expand Up @@ -764,6 +765,12 @@ impl LocalExec {
)
.expect("Failed to create gas status")
};
let gas_data = GasData {
payment: tx_info.gas.clone(),
owner: tx_info.gas_owner.unwrap_or(tx_info.sender),
price: tx_info.gas_price,
budget: tx_info.gas_budget,
};
let (inner_store, gas_status, effects, _timings, result) = executor
.execute_transaction_to_effects(
&self,
Expand All @@ -774,7 +781,7 @@ impl LocalExec {
&tx_info.executed_epoch,
tx_info.epoch_start_timestamp,
CheckedInputObjects::new_for_replay(input_objects.clone()),
tx_info.gas.clone(),
gas_data,
gas_status,
transaction_kind.clone(),
tx_info.sender,
Expand Down Expand Up @@ -823,6 +830,12 @@ impl LocalExec {
trace!(target: "replay_gas_info", "{}", Pretty(gas_status));

let skip_checks = true;
let gas_data = GasData {
payment: tx_info.gas.clone(),
owner: tx_info.gas_owner.unwrap_or(tx_info.sender),
price: tx_info.gas_price,
budget: tx_info.gas_budget,
};
if let ProgrammableTransaction(pt) = transaction_kind {
trace!(
target: "replay_ptb_info",
Expand All @@ -841,7 +854,7 @@ impl LocalExec {
&tx_info.executed_epoch,
tx_info.epoch_start_timestamp,
CheckedInputObjects::new_for_replay(input_objects),
tx_info.gas.clone(),
gas_data,
SuiGasStatus::new(
tx_info.gas_budget,
tx_info.gas_price,
Expand Down Expand Up @@ -924,7 +937,7 @@ impl LocalExec {
reference_gas_price,
)
.unwrap();
let (kind, signer, gas) = executable.transaction_data().execution_parts();
let (kind, signer, gas_data) = executable.transaction_data().execution_parts();
let executor = sui_execution::executor(&protocol_config, true, None).unwrap();
let (_, _, effects, _timings, exec_res) = executor.execute_transaction_to_effects(
&store,
Expand All @@ -935,7 +948,7 @@ impl LocalExec {
&executed_epoch,
epoch_start_timestamp,
input_objects,
gas,
gas_data,
gas_status,
kind,
signer,
Expand Down Expand Up @@ -1530,8 +1543,9 @@ impl LocalExec {
input_objects: input_objs,
shared_object_refs,
gas: gas_object_refs,
gas_budget: gas_data.budget,
gas_owner: (gas_data.owner != sender).then_some(gas_data.owner),
gas_price: gas_data.price,
gas_budget: gas_data.budget,
executed_epoch: epoch_id,
dependencies: effects.dependencies().to_vec(),
effects: SuiTransactionBlockEffects::V1(effects),
Expand Down Expand Up @@ -1592,8 +1606,6 @@ impl LocalExec {
}
})
.collect();
let gas_data = orig_tx.transaction_data().gas_data();
let gas_object_refs: Vec<_> = gas_data.clone().payment.into_iter().collect();
let receiving_objs = orig_tx
.transaction_data()
.receiving_objects()
Expand All @@ -1611,6 +1623,8 @@ impl LocalExec {
let (epoch_start_timestamp, reference_gas_price) = self
.get_epoch_start_timestamp_and_rgp(epoch_id, tx_digest)
.await?;
let gas_data = orig_tx.transaction_data().gas_data();
let gas_object_refs: Vec<_> = gas_data.clone().payment.into_iter().collect();

Ok(OnChainTransactionInfo {
kind: tx_kind_orig.clone(),
Expand All @@ -1619,8 +1633,9 @@ impl LocalExec {
input_objects: input_objs,
shared_object_refs,
gas: gas_object_refs,
gas_budget: gas_data.budget,
gas_owner: (gas_data.owner != sender).then_some(gas_data.owner),
gas_price: gas_data.price,
gas_budget: gas_data.budget,
executed_epoch: epoch_id,
dependencies: effects.dependencies().to_vec(),
effects,
Expand Down
2 changes: 2 additions & 0 deletions crates/sui-replay/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ pub struct OnChainTransactionInfo {
pub modified_at_versions: Vec<(ObjectID, SequenceNumber)>,
pub shared_object_refs: Vec<ObjectRef>,
pub gas: Vec<(ObjectID, SequenceNumber, ObjectDigest)>,
#[serde(default)]
pub gas_owner: Option<SuiAddress>,
pub gas_budget: u64,
pub gas_price: u64,
pub executed_epoch: u64,
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-single-node-benchmark/src/single_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ impl SingleValidator {
self.epoch_store.reference_gas_price(),
)
.unwrap();
let (kind, signer, gas) = executable.transaction_data().execution_parts();
let (kind, signer, gas_data) = executable.transaction_data().execution_parts();
let (inner_temp_store, _, effects, _timings, _) =
self.epoch_store.executor().execute_transaction_to_effects(
&store,
Expand All @@ -215,7 +215,7 @@ impl SingleValidator {
&self.epoch_store.epoch(),
0,
input_objects,
gas,
gas_data,
gas_status,
kind,
signer,
Expand Down
5 changes: 3 additions & 2 deletions crates/sui-swarm-config/src/network_config_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,8 @@ mod test {
let certificate_deny_set = HashSet::new();
let epoch = EpochData::new_test();
let transaction_data = &genesis_transaction.data().intent_message().value;
let (kind, signer, _) = transaction_data.execution_parts();
let (kind, signer, mut gas_data) = transaction_data.execution_parts();
gas_data.payment = vec![];
let input_objects = CheckedInputObjects::new_for_genesis(vec![]);

let (_inner_temp_store, _, effects, _timings, _execution_error) = executor
Expand All @@ -612,7 +613,7 @@ mod test {
&epoch.epoch_id(),
epoch.epoch_start_timestamp(),
input_objects,
vec![],
gas_data,
SuiGasStatus::new_unmetered(),
kind,
signer,
Expand Down
Loading

0 comments on commit 447b0a3

Please sign in to comment.