Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commit per epoch config load to effects v2 #18501

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ A: object(0,0), B: object(0,1)
task 1 'publish'. lines 12-34:
created: object(1,0), object(1,1), object(1,2), object(1,3), object(1,4), object(1,5)
mutated: object(0,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 18316000, storage_rebate: 0, non_refundable_storage_fee: 0

task 2 'view-object'. lines 36-36:
Expand Down Expand Up @@ -133,6 +134,7 @@ Contents: sui::coin::TreasuryCap<test::regulated_coin::REGULATED_COIN> {
task 8 'run'. lines 49-51:
created: object(8,0)
mutated: object(0,0), object(1,1)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 3936800, storage_rebate: 2437776, non_refundable_storage_fee: 24624

task 9 'run'. lines 52-54:
Expand All @@ -153,4 +155,5 @@ gas summary: computation_cost: 1000000, storage_cost: 9522800, storage_rebate:

task 13 'transfer-object'. lines 64-64:
mutated: object(0,1), object(8,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 2462400, storage_rebate: 1459656, non_refundable_storage_fee: 14744
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ A: object(0,0)
task 1 'publish'. lines 9-54:
created: object(1,0), object(1,1), object(1,2), object(1,3), object(1,4), object(1,5), object(1,6), object(1,7), object(1,8), object(1,9), object(1,10)
mutated: object(0,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 33082800, storage_rebate: 0, non_refundable_storage_fee: 0

task 2 'view-object'. lines 56-56:
Expand Down Expand Up @@ -259,4 +260,5 @@ Error: Error checking transaction input objects: AddressDeniedForCoin { address:

task 15 'transfer-object'. lines 85-85:
mutated: object(0,0), object(1,2)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 2416800, storage_rebate: 2392632, non_refundable_storage_fee: 24168
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ A: object(0,0)
task 1 'publish'. lines 8-46:
created: object(1,0), object(1,1), object(1,2), object(1,3), object(1,4), object(1,5), object(1,6)
mutated: object(0,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 21766400, storage_rebate: 0, non_refundable_storage_fee: 0

task 2 'view-object'. lines 48-48:
Expand Down Expand Up @@ -156,4 +157,5 @@ gas summary: computation_cost: 1000000, storage_cost: 9522800, storage_rebate:

task 12 'run'. lines 72-72:
mutated: object(0,0), object(1,0), object(1,2)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 3807600, storage_rebate: 3769524, non_refundable_storage_fee: 38076
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ A: object(0,0), B: object(0,1)
task 1 'publish'. lines 12-37:
created: object(1,0), object(1,1), object(1,2), object(1,3), object(1,4), object(1,5)
mutated: object(0,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 18392000, storage_rebate: 0, non_refundable_storage_fee: 0

task 2 'run'. lines 38-40:
created: object(2,0)
mutated: object(0,0), object(1,1)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 3936800, storage_rebate: 2437776, non_refundable_storage_fee: 24624

task 3 'run'. lines 41-43:
Expand All @@ -22,6 +24,7 @@ gas summary: computation_cost: 1000000, storage_cost: 12190400, storage_rebate:
task 4 'run'. lines 44-44:
created: object(4,0)
mutated: object(0,0), object(1,1)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 3936800, storage_rebate: 2437776, non_refundable_storage_fee: 24624

task 5 'advance-epoch'. lines 46-48:
Expand All @@ -45,4 +48,5 @@ Epoch advanced: 2
task 10 'run'. lines 60-60:
created: object(10,0)
mutated: object(0,0), object(1,1)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 3936800, storage_rebate: 2437776, non_refundable_storage_fee: 24624
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ A: object(0,0), B: object(0,1)
task 1 'publish'. lines 13-48:
created: object(1,0), object(1,1), object(1,2), object(1,3), object(1,4), object(1,5)
mutated: object(0,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 19471200, storage_rebate: 0, non_refundable_storage_fee: 0

task 2 'view-object'. lines 49-51:
Expand All @@ -23,6 +24,7 @@ Contents: sui::coin::DenyCapV2<test::regulated_coin::REGULATED_COIN> {
task 3 'run'. lines 52-54:
created: object(3,0)
mutated: object(0,0), object(1,1)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 3936800, storage_rebate: 2437776, non_refundable_storage_fee: 24624

task 4 'run'. lines 55-57:
Expand Down Expand Up @@ -54,4 +56,5 @@ gas summary: computation_cost: 1000000, storage_cost: 988000, storage_rebate: 9

task 10 'transfer-object'. lines 73-73:
mutated: object(0,1), object(3,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 2462400, storage_rebate: 1459656, non_refundable_storage_fee: 14744
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,25 @@ A: object(0,0), B: object(0,1)
task 1 'publish'. lines 10-72:
created: object(1,0), object(1,1), object(1,2), object(1,3), object(1,4), object(1,5)
mutated: object(0,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 21964000, storage_rebate: 0, non_refundable_storage_fee: 0

task 2 'run'. lines 73-75:
created: object(2,0)
mutated: object(0,0), object(1,1)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 3936800, storage_rebate: 2437776, non_refundable_storage_fee: 24624

task 3 'run'. lines 76-78:
created: object(3,0)
mutated: object(0,0), object(1,1)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 4119200, storage_rebate: 2437776, non_refundable_storage_fee: 24624

task 4 'run'. lines 79-81:
created: object(4,0)
mutated: object(0,0), object(1,1)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 4119200, storage_rebate: 2437776, non_refundable_storage_fee: 24624

task 5 'run'. lines 82-84:
Expand All @@ -47,6 +51,7 @@ task 10 'run'. lines 98-98:
mutated: object(0,0)
unwrapped: object(10,0)
deleted: object(3,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 2462400, storage_rebate: 2618352, non_refundable_storage_fee: 26448

task 11 'advance-epoch'. lines 100-103:
Expand Down Expand Up @@ -94,4 +99,5 @@ task 21 'run'. lines 131-131:
mutated: object(0,0)
unwrapped: object(1,1)
deleted: object(18,0)
unchanged_shared: 0x0000000000000000000000000000000000000000000000000000000000000403
gas summary: computation_cost: 1000000, storage_cost: 2462400, storage_rebate: 2618352, non_refundable_storage_fee: 26448
2 changes: 2 additions & 0 deletions crates/sui-core/tests/staged/sui.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,8 @@ UnchangedSharedKind:
Cancelled:
NEWTYPE:
TYPENAME: SequenceNumber
4:
PerEpochConfig: UNIT
UpgradeInfo:
STRUCT:
- upgraded_id:
Expand Down
44 changes: 25 additions & 19 deletions crates/sui-types/src/deny_list_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@
use crate::base_types::{EpochId, ObjectID, SuiAddress};
use crate::config::{Config, Setting};
use crate::deny_list_v1::{
get_deny_list_root_object, input_object_coin_types_for_denylist_check,
DENY_LIST_COIN_TYPE_INDEX, DENY_LIST_MODULE,
input_object_coin_types_for_denylist_check, DENY_LIST_COIN_TYPE_INDEX, DENY_LIST_MODULE,
};
use crate::dynamic_field::{get_dynamic_field_from_store, DOFWrapper};
use crate::error::{ExecutionError, ExecutionErrorKind, UserInputError, UserInputResult};
use crate::id::UID;
use crate::object::Object;
use crate::storage::ObjectStore;
use crate::transaction::{CheckedInputObjects, ReceivingObjects};
use crate::{MoveTypeTagTrait, SUI_FRAMEWORK_PACKAGE_ID};
use crate::{MoveTypeTagTrait, SUI_DENY_LIST_OBJECT_ID, SUI_FRAMEWORK_PACKAGE_ID};
use move_core_types::ident_str;
use move_core_types::language_storage::{StructTag, TypeTag};
use serde::de::DeserializeOwned;
Expand Down Expand Up @@ -120,11 +119,13 @@ pub fn check_coin_deny_list_v2_during_signing(
Ok(())
}

/// Returns 1) whether the coin deny list check passed, and 2) the number of regulated transfers.
pub fn check_coin_deny_list_v2_during_execution(
written_objects: &BTreeMap<ObjectID, Object>,
cur_epoch: EpochId,
object_store: &dyn ObjectStore,
) -> Result<(), ExecutionError> {
) -> (Result<(), ExecutionError>, u64) {
let mut num_regulated_transfers = 0;
let mut new_coin_owners = BTreeMap::new();
for obj in written_objects.values() {
if obj.is_gas_coin() {
Expand All @@ -140,38 +141,43 @@ pub fn check_coin_deny_list_v2_during_execution(
.entry(coin_type.to_canonical_string(false))
.or_insert_with(BTreeSet::new)
.insert(owner);
num_regulated_transfers += 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this incremented here, instead of after line 149?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, but then we would need to add the size of the owners there. This is a lot simpler.

}
for (coin_type, owners) in new_coin_owners {
let Some(deny_list) = get_per_type_coin_deny_list_v2(&coin_type, object_store) else {
continue;
};
if check_global_pause(&deny_list, object_store, Some(cur_epoch)) {
return Err(ExecutionError::new(
ExecutionErrorKind::CoinTypeGlobalPause { coin_type },
None,
));
return (
Err(ExecutionError::new(
ExecutionErrorKind::CoinTypeGlobalPause { coin_type },
None,
)),
num_regulated_transfers,
);
}
for owner in owners {
if check_address_denied_by_config(&deny_list, owner, object_store, Some(cur_epoch)) {
return Err(ExecutionError::new(
ExecutionErrorKind::AddressDeniedForCoin {
address: owner,
coin_type,
},
None,
));
return (
Err(ExecutionError::new(
ExecutionErrorKind::AddressDeniedForCoin {
address: owner,
coin_type,
},
None,
)),
num_regulated_transfers,
);
}
}
}
Ok(())
(Ok(()), num_regulated_transfers)
}

pub fn get_per_type_coin_deny_list_v2(
coin_type: &String,
object_store: &dyn ObjectStore,
) -> Option<Config> {
let deny_list_root =
get_deny_list_root_object(object_store).expect("Deny list root object not found");
let config_key = DOFWrapper {
name: ConfigKey {
per_type_index: DENY_LIST_COIN_TYPE_INDEX,
Expand All @@ -180,7 +186,7 @@ pub fn get_per_type_coin_deny_list_v2(
};
// TODO: Consider caching the config object UID to avoid repeat deserialization.
let config: Config =
get_dynamic_field_from_store(object_store, deny_list_root.id(), &config_key).ok()?;
get_dynamic_field_from_store(object_store, SUI_DENY_LIST_OBJECT_ID, &config_key).ok()?;
Some(config)
}

Expand Down
48 changes: 31 additions & 17 deletions crates/sui-types/src/effects/effects_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ use crate::gas::GasCostSummary;
use crate::is_system_package;
use crate::object::{Owner, OBJECT_START_VERSION};
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
#[cfg(debug_assertions)]
use std::collections::HashSet;
use std::collections::{BTreeMap, BTreeSet};

/// The response from processing a transaction or a certified transaction
#[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -107,22 +107,28 @@ impl TransactionEffectsAPI for TransactionEffectsV2 {
}
_ => None,
})
.chain(self.unchanged_shared_objects.iter().map(
|(id, change_kind)| match change_kind {
UnchangedSharedKind::ReadOnlyRoot((version, digest)) => {
InputSharedObject::ReadOnly((*id, *version, *digest))
}
UnchangedSharedKind::MutateDeleted(seqno) => {
InputSharedObject::MutateDeleted(*id, *seqno)
}
UnchangedSharedKind::ReadDeleted(seqno) => {
InputSharedObject::ReadDeleted(*id, *seqno)
}
UnchangedSharedKind::Cancelled(seqno) => {
InputSharedObject::Cancelled(*id, *seqno)
}
},
))
.chain(
self.unchanged_shared_objects
.iter()
.filter_map(|(id, change_kind)| match change_kind {
UnchangedSharedKind::ReadOnlyRoot((version, digest)) => {
Some(InputSharedObject::ReadOnly((*id, *version, *digest)))
}
UnchangedSharedKind::MutateDeleted(seqno) => {
Some(InputSharedObject::MutateDeleted(*id, *seqno))
}
UnchangedSharedKind::ReadDeleted(seqno) => {
Some(InputSharedObject::ReadDeleted(*id, *seqno))
}
UnchangedSharedKind::Cancelled(seqno) => {
Some(InputSharedObject::Cancelled(*id, *seqno))
}
// We can not expose the per epoch config object as input shared object,
// since it does not require sequencing, and hence shall not be considered
// as a normal input shared object.
UnchangedSharedKind::PerEpochConfig => None,
}),
)
.collect()
}

Expand Down Expand Up @@ -405,6 +411,7 @@ impl TransactionEffectsV2 {
executed_epoch: EpochId,
gas_used: GasCostSummary,
shared_objects: Vec<SharedInput>,
loaded_per_epoch_config_objects: BTreeSet<ObjectID>,
transaction_digest: TransactionDigest,
lamport_version: SequenceNumber,
changed_objects: BTreeMap<ObjectID, EffectsObjectChange>,
Expand Down Expand Up @@ -435,6 +442,11 @@ impl TransactionEffectsV2 {
Some((id, UnchangedSharedKind::Cancelled(version)))
}
})
.chain(
loaded_per_epoch_config_objects
.into_iter()
.map(|id| (id, UnchangedSharedKind::PerEpochConfig)),
)
.collect();
let changed_objects: Vec<_> = changed_objects.into_iter().collect();

Expand Down Expand Up @@ -595,4 +607,6 @@ pub enum UnchangedSharedKind {
ReadDeleted(SequenceNumber),
/// Shared objects in cancelled transaction. The sequence number embed cancellation reason.
Cancelled(SequenceNumber),
/// Read of a per-epoch config object that should remain the same during an epoch.
PerEpochConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

this requires a new protocol version, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The per epoch config object is only checked when coin deny list v2 is enabled, and hence is already behind a new protocol version.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it

}
11 changes: 7 additions & 4 deletions crates/sui-types/src/effects/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use enum_dispatch::enum_dispatch;
pub use object_change::{EffectsObjectChange, ObjectIn, ObjectOut};
use serde::{Deserialize, Serialize};
use shared_crypto::intent::{Intent, IntentScope};
use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};
pub use test_effects_builder::TestEffectsBuilder;

mod effects_v1;
Expand Down Expand Up @@ -126,6 +126,7 @@ impl TransactionEffects {
executed_epoch: EpochId,
gas_used: GasCostSummary,
shared_objects: Vec<SharedInput>,
loaded_per_epoch_config_objects: BTreeSet<ObjectID>,
transaction_digest: TransactionDigest,
lamport_version: SequenceNumber,
changed_objects: BTreeMap<ObjectID, EffectsObjectChange>,
Expand All @@ -138,6 +139,7 @@ impl TransactionEffects {
executed_epoch,
gas_used,
shared_objects,
loaded_per_epoch_config_objects,
transaction_digest,
lamport_version,
changed_objects,
Expand Down Expand Up @@ -313,11 +315,12 @@ pub trait TransactionEffectsAPI {
/// It includes objects that are mutated, wrapped and deleted.
/// This API is only available on effects v2 and above.
fn old_object_metadata(&self) -> Vec<(ObjectRef, Owner)>;
/// Returns the list of shared objects used in the input, with full object reference
/// and use kind. This is needed in effects because in transaction we only have object ID
/// Returns the list of sequenced shared objects used in the input.
/// This is needed in effects because in transaction we only have object ID
/// for shared objects. Their version and digest can only be figured out after sequencing.
/// Also provides the use kind to indicate whether the object was mutated or read-only.
/// Down the road it could also indicate use-of-deleted.
/// It does not include per epoch config objects since they do not require sequencing.
/// TODO: Rename this function to indicate sequencing requirement.
fn input_shared_objects(&self) -> Vec<InputSharedObject>;
fn created(&self) -> Vec<(ObjectRef, Owner)>;
fn mutated(&self) -> Vec<(ObjectRef, Owner)>;
Expand Down
3 changes: 2 additions & 1 deletion crates/sui-types/src/effects/test_effects_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::gas::GasCostSummary;
use crate::message_envelope::Message;
use crate::object::Owner;
use crate::transaction::{InputObjectKind, SenderSignedData, TransactionDataAPI};
use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};

pub struct TestEffectsBuilder {
transaction: SenderSignedData,
Expand Down Expand Up @@ -139,6 +139,7 @@ impl TestEffectsBuilder {
executed_epoch,
GasCostSummary::default(),
shared_objects,
BTreeSet::new(),
self.transaction.digest(),
lamport_version,
changed_objects,
Expand Down
Loading
Loading