Create a LayoutResolver to compute layout from VM types #11529

dariorussi · 2023-04-30T22:40:46Z

Description

Move expensive_check_sui_conservation to node init
Expose a layout resolver based on a VM session
Decouple MoveResolver traits from Storage traits

In some of the places where we want to use the layout resolver, we
don't have access to all the information to implement StorageView,
but we only need to use the BackingPackageStore.

To support these cases, we need to provide a state view to
LinkageView that can serve packages, and then shim the Module and
Resource resolution functions.

This requires that LinkageView no longer accept a reference
parameter, but a value (so we can inject state into it) -- similar to
the trick we pulled to allow Session to own a LinkageView. Then
implementors (like TemporaryStore) that only need to be passed by
reference can be passed by explicit reference,

i.e. LinkageView<'state, TemporaryStore> becomes
LinkageView<&'state TemporaryStore>.

However, we cannot impose a &'state S: StorageView bound, because
some APIs in StorageView require a mutable reference, so we needed
to decouple the resolution traits (now SuiResolver) and the storage
traits, to impose twin constraints:

fn ...<'state, S>
where
  S: StorageView
  &'state S: SuiResolver

Logging improvements
Bring invariant_violation! macro into sui-types.
Create a variant of it -- make_invariant_violation! -- that
creates the error, but does not wrap it in a result of return it.
Add format! string support to these macros
Use this macro for invariant violations within conservation checking
with extra context on what is failing (what stage, what type is
involved)
Enable telemetry_subscription in the move_package_upgrade_tests to
see the logs from authorities during tests.
(Temporary) Print the IDs of published/upgraded packages in the
failing test, for context.

Test Plan

Existing tests

If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

user-visible impact
breaking change for a client SDKs
breaking change for FNs (FN binary must upgrade)
breaking change for validators or node operators (must upgrade binaries)
breaking change for on-chain data layout
necessitate either a data wipe or data migration

Release notes

vercel · 2023-04-30T22:40:56Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments

Name	Status	Updated (UTC)
explorer	⬜️ Ignored (Inspect)	May 10, 2023 10:14pm
explorer-storybook	⬜️ Ignored (Inspect)	May 10, 2023 10:14pm
sui-wallet-kit	⬜️ Ignored (Inspect)	May 10, 2023 10:14pm
wallet-adapter	⬜️ Ignored (Inspect)	May 10, 2023 10:14pm

lxfind · 2023-04-30T22:52:23Z

crates/sui-node/src/lib.rs

+        // an epoch and the SUI conservation check will fail. This also initialize
+        // the expected_network_sui_amount table.
+        store
+            .expensive_check_sui_conservation()


I don't understand this change. As in the comment, this is only safe to call at genesis (i.e. when the db table is empty). If it's not genesis, it could be in the middle of an epoch and it does not guarantee conservation at all.

Is there a way to check for genesis at this point, or that the DB was otherwise opened? @dariorussi, the thing we missed when looking at the code yesterday is that within AuthorityStore::open_inner this call is guarded by a check that the database was previously empty, implying that the node is starting from scratch, and we will need some way to recreate that check at the new location:

sui/crates/sui-core/src/authority/authority_store.rs

Lines 244 to 248 in 250221a

// Only initialize an empty database.

if store

.database_is_empty()

.expect("Database read should not fail at init.")

{

The reason we need to make this change is that the new implementation of the check will require access to the VM which is created as part of the AuthorityPerEpochStore, (which itself is created after the AuthorityStore::open that used to perform this check).

let me look at this again then, I guess it has to be protected...

@amnn I noticed :) and looking into how to workaround that.
@lxfind I think I can move the creation of the AuthorityPerpetualTables out of AuthorityStore and do the check there (before calling into AuthorityStore) and protect the check that way for both prod and testing.
I'll have code up soon, but the question @amnn asked is fair, if there was a simpler way

@lxfind, Can we rely on the fact that the checks themselves use their own DB and initialise it if it's empty?

sui/crates/sui-core/src/authority/authority_store.rs

Lines 1599 to 1619 in 250221a

if let Some(expected_imbalance) = self

.perpetual_tables

.expected_storage_fund_imbalance

.get(&())

.expect("DB read cannot fail")

{

fp_ensure!(

imbalance == expected_imbalance,

SuiError::from(

format!(

"Inconsistent state detected at epoch {}: total storage rebate: {}, storage fund balance: {}, expected imbalance: {}",

system_state.epoch, total_storage_rebate, storage_fund_balance, expected_imbalance

).as_str()

)

);

} else {

self.perpetual_tables

.expected_storage_fund_imbalance

.insert(&(), &imbalance)

.expect("DB write cannot fail");

}

@lxfind, Can we rely on the fact that the checks themselves use their own DB and initialise it if it's empty?

Not sure exactly what you meant.
But the earlier proposal of creating tables outside sounds reasonable thing to try

Sorry, I meant "table" there instead of "DB". I was referring to the fact that within the expensive check, there is a conditional (linked above) that checks whether there are already entries for expected_storage_fund_imbalance (and expected_network_sui_amount) in the perpetual tables. If there are, then it does a conservation check, and if there are not, it initialises those tables.

If it's not genesis, it could be in the middle of an epoch and it does not guarantee conservation at all.

This bit still doesn't make sense to me: How do we lose the conservation guarantee, assuming that even if the node is started mid-epoch, we initialise its conservation-related tables before it starts processing any transactions?

I'm assuming here that as long as we don't have any transactions in flight, we would expect the live object set at any time during an epoch to conserve SUI as well.

The missing pieces are the gas rewards that's accumulated in Rust/db, not on-chain.
So if we do conservation check in the middle of an epoch, it's going to be very tricky to figure out how much gas reward that has accumulated so far.

amnn · 2023-04-30T23:17:44Z

(The change to introduce the new Session API is otherwise good though)

lxfind · 2023-05-01T00:07:31Z

I assume we cannot just create a new VM on-the-fly for the conservation check?

lxfind · 2023-05-01T00:15:32Z

By the way, the primary conservation check is in fact called during reconfiguration. The one at genesis is just to initialize it. We will need to deal with both if we need to make changes.

dariorussi · 2023-05-01T00:30:10Z

crates/sui-core/src/authority/test_authority_builder.rs

@@ -162,6 +175,16 @@ impl<'a> TestAuthorityBuilder<'a> {
            &ExpensiveSafetyCheckConfig::default(),
        );

+        if empty_database {


I am assuming we want this in testing as well?
is this too much of a change?

dariorussi · 2023-05-01T00:30:36Z

crates/sui-node/src/lib.rs

@@ -248,6 +255,16 @@ impl SuiNode {
            &config.expensive_safety_check_config,
        );

+        if empty_database {


reasonable?

dariorussi · 2023-05-01T00:34:20Z

By the way, the primary conservation check is in fact called during reconfiguration. The one at genesis is just to initialize it. We will need to deal with both if we need to make changes.

correct and the one in reconfiguration is in authority so with "easy" access to the VM.
The call while creating the AuthorityStore is the "complicated" one

dariorussi · 2023-05-01T00:34:57Z

made a change to create perpetual tables outside the AuthorityStore, let's see if it seems reasonable.
Thanks!

amnn · 2023-05-01T00:39:20Z

I assume we cannot just create a new VM on-the-fly for the conservation check?

We can do this, but then it may involve doing a lot of extra work to load the relevant packages into the VM during the conservation check that we would otherwise avoid by using the same VM.

dariorussi · 2023-05-01T00:45:58Z

I assume we cannot just create a new VM on-the-fly for the conservation check?

We can do this, but then it may involve doing a lot of extra work to load the relevant packages into the VM during the conservation check that we would otherwise avoid by using the same VM.

right, as @amnn said we thought it was a bit too much and it was nice to use the same VM as what is created for the authority state.
It also played a role that the solution we are implementing is a "temporary" solution until we review the layout story entirely

lxfind · 2023-05-01T00:47:00Z

We can do this, but then it may involve doing a lot of extra work to load the relevant packages into the VM during the conservation check that we would otherwise avoid by using the same VM.

It uses an internal module cache so it might not be too bad

amnn · 2023-05-01T00:47:41Z

It does, but it will entirely replicate that inside the VM, multiple times over.

lxfind

Looks reasonable change to me. Now that we have empty_table outside there, I wonder if it could be used to simplify other things (not sure, but worth a quick check)

lxfind · 2023-05-01T00:49:22Z

crates/sui-node/src/lib.rs

            &config.db_path().join("store"),
            Some(perpetual_options.options),
+        ));
+        let empty_database = perpetual_tables


Maybe leave a comment here saying that empty database indicates genesis

Or maybe just rename the variable to is_genesis?

dariorussi · 2023-05-01T00:52:10Z

Looks reasonable change to me. Now that we have empty_table outside there, I wonder if it could be used to simplify other things (not sure, but worth a quick check)

let's chat tomorrow about it as I keep going with the implementation of the overall change.
We will also take a look at what else could be simplified

amnn

Looks good so far, just reviewing along the way!

amnn · 2023-05-02T16:06:15Z

crates/sui-adapter/src/type_layout_resolver.rs

+        let ty = match load_type(&mut self.session, &type_tag) {
+            Err(_) => {
+                return Err(SuiError::FailObjectLayout {
+                    st: format!("{}", struct_tag),
+                })
+            }
+            Ok(ty) => ty,
+        };


nit: let ... else, although, is it worth including the error as well?

Suggested change

let ty = match load_type(&mut self.session, &type_tag) {

Err(_) => {

return Err(SuiError::FailObjectLayout {

st: format!("{}", struct_tag),

})

}

Ok(ty) => ty,

};

let Ok(ty) = load_type(&mut self.session, &type_tag) else {

return Err(SuiError::FailObjectLayout { st: format!("{}", struct_tag) });

};

amnn · 2023-05-02T16:08:12Z

crates/sui-adapter/src/type_layout_resolver.rs

+        match layout {
+            Err(_) => Err(SuiError::FailObjectLayout {
+                st: format!("{}", struct_tag),
+            }),
+            Ok(type_layout) => match type_layout {
+                MoveTypeLayout::Struct(layout) => Ok(layout),
+                _ => Err(SuiError::FailObjectLayout {
+                    st: format!("{}", struct_tag),
+                }),
+            },
+        }


nit, same deal:

Suggested change

match layout {

Err(_) => Err(SuiError::FailObjectLayout {

st: format!("{}", struct_tag),

}),

Ok(type_layout) => match type_layout {

MoveTypeLayout::Struct(layout) => Ok(layout),

_ => Err(SuiError::FailObjectLayout {

st: format!("{}", struct_tag),

}),

},

}

let Ok(MoveTypeLayout::Struct(layout)) = layout else {

return Err(SuiError::FailObjectLayout {

st: format!("{}", struct_tag),

})

}

amnn · 2023-05-02T16:20:47Z

crates/sui-node/src/lib.rs

            &config.db_path().join("store"),
            Some(perpetual_options.options),
+        ));
+        let empty_database = perpetual_tables


Or maybe just rename the variable to is_genesis?

amnn · 2023-05-02T16:23:32Z

crates/sui-types/src/layout_resolver.rs

+pub trait LayoutResolver {
+    fn get_layout(
+        &mut self,
+        object: &MoveObject,


nit: Should this accept the full object, or just its type?

not sure maybe both API in time?

leaving it as is for now

amnn · 2023-05-02T16:26:54Z

crates/sui-types/src/temporary_store.rs

@@ -1494,7 +1512,11 @@ impl<S: ChildObjectResolver> Storage for TemporaryStore<S> {

 impl<S: BackingPackageStore> BackingPackageStore for TemporaryStore<S> {
    fn get_package_object(&self, package_id: &ObjectID) -> SuiResult<Option<Object>> {
-        self.store.get_package_object(package_id)


I don't know if this change is going to have broader implications -- I think it might inadvertantly introduce incomplete support for calling into packages that you just published. It would be better to limit this support to some kind of wrapper type that we only use for our purposes.

Please double check where this is used to make sure the semantic change is ok

yeah I agree on both and meant to flag it in the PR but figured I'd mention f2f.
This was vital to make things work and I wondered if that was a good thing or not.
We can do the wrapper and I'll review the usage, meanwhile adding @sblackshear who may have introduced this originally

@awelc thoughts about this change?

lxfind · 2023-05-02T16:41:01Z

crates/sui-core/src/authority/authority_store.rs

@@ -1533,6 +1521,9 @@ impl AuthorityStore {
                            let mut task_objects = vec![];
                            mem::swap(&mut pending_objects, &mut task_objects);
                            let package_cache_clone = package_cache.clone();
+                            // TODO: hook up the TypeLayoutResolver. It may require some traits implementation


Finish this TODO?

yeah this needs to be fixed and working on it, it'll go away soon

lxfind · 2023-05-02T16:45:33Z

crates/sui-core/src/authority/test_authority_builder.rs

            None => {
+                let perpetual_tables =
+                    Arc::new(AuthorityPerpetualTables::open(&path.join("store"), None));
+                let empty_database = perpetual_tables


I don't think we need this in test authority builder.
Test authority builder is used to create a single authority, which won't be able to run epoch-conservation check anyway.

ok so removing the change for this file entirely

lxfind · 2023-05-02T16:46:50Z

crates/sui-types/src/temporary_store.rs

@@ -1494,7 +1512,11 @@ impl<S: ChildObjectResolver> Storage for TemporaryStore<S> {

 impl<S: BackingPackageStore> BackingPackageStore for TemporaryStore<S> {
    fn get_package_object(&self, package_id: &ObjectID) -> SuiResult<Option<Object>> {
-        self.store.get_package_object(package_id)


Please double check where this is used to make sure the semantic change is ok

dariorussi · 2023-05-09T23:40:43Z

ok this is ready so let's try to get a review and make sure everything is good

crates/sui-adapter/src/type_layout_resolver.rs

awelc · 2023-05-10T00:00:47Z

crates/sui-adapter/src/type_layout_resolver.rs

+        let ty = match load_type(&mut self.session, &type_tag) {
+            Ok(ty) => ty,
+            Err(err) => {
+                println!("ERROR: {:?}", err);


Are we keeping the println?

no, thanks for pointing it out!

awelc · 2023-05-10T00:04:08Z

crates/sui-core/src/authority.rs

@@ -1697,7 +1699,16 @@ impl AuthorityState {
            })?;

        let layout = match request.object_format_options {
-            Some(format) => object.get_layout(format, epoch_store.module_cache().as_ref())?,
+            Some(format) => match object.data.try_as_move() {


Nit. Use map on the Option instead of match (in both cases)?

I'll think about it, thanks

crates/sui-core/src/authority.rs

awelc · 2023-05-10T00:15:47Z

crates/sui-types/src/temporary_store.rs

+    /// This function is intended to be called *after* we have charged for gas + applied the storage
+    /// rebate to the gas object, but *before* we have updated object versions.  If
+    /// `do_expensive_checks` is false, this function will only check conservation of object storage
+    /// rea `epoch_fees` and `epoch_rebates` are only set for advance epoch transactions.  The


I wish we did not lint this comment and left it alone.... lol....

This is my fault -- I just couldn't read it how it was before. I couldn't figure out what "rea" meant before, my guess is "rebates", but I didn't want to make the edit based on a guess, so I left it as it was.

amnn

LGTM, thanks for working on this @dariorussi !

amnn · 2023-05-10T14:29:31Z

crates/sui-adapter/src/type_layout_resolver.rs

+        let ty = match load_type(&mut self.session, &type_tag) {
+            Ok(ty) => ty,
+            Err(_err) => {
+                return Err(SuiError::FailObjectLayout {
+                    st: format!("{}", struct_tag),
+                });
+            }
+        };


nit: another opportunity for let ... else

Suggested change

let ty = match load_type(&mut self.session, &type_tag) {

Ok(ty) => ty,

Err(_err) => {

return Err(SuiError::FailObjectLayout {

st: format!("{}", struct_tag),

});

}

};

let Ok(ty) = load_type(&mut self.session, &type_tag) else {

return Err(SuiError::FailObjectLayout {

st: format!("{}", struct_tag),

});

};

amnn · 2023-05-10T14:34:33Z

crates/sui-core/src/authority.rs

        let layout = match request.object_format_options {
-            Some(format) => object.get_layout(format, epoch_store.module_cache().as_ref())?,
+            Some(format) => match object.data.try_as_move() {
+                None => None,
+                Some(move_obj) => {
+                    let epoch_store = self.load_epoch_store_one_call_per_task();
+                    let move_vm = epoch_store.move_vm();
+                    let mut layout_resolver =
+                        TypeLayoutResolver::new(move_vm, self.database.as_ref());
+                    Some(layout_resolver.get_layout(move_obj, format)?)
+                }
+            },
            None => None,
        };


Or alternatively, match on both optional values in one go, in an if let:

let layout = if let (Some(format), Some(move_obj) = ( request.object.object_format_options, object.data.try_as_move(), ) { let epoch_store = self.load_epoch_store_one_call_per_task(); let move_vm = epoch_store.move_vm(); let mut layout_resolver = TypeLayoutResolver::new(move_vm, self.database.as_ref()); Some(layout_resolver.get_layout(move_obj, format)?) } else { None }

I do not know... you guys are really obsessed with removing matches.... :)
let me try in the code base and see if I like it

I like match, promise! I tend to use it when the match arms are symmetrical (i.e. not a happy path + error case), and there are more than two. But in other cases, I find them less clear than if let or let ... else.

amnn · 2023-05-10T14:36:04Z

crates/sui-core/src/authority.rs

-                                // threading the epoch_store through this API does not
-                                // seem possible, so we just read it from the state (self) and fetch
-                                // the module cache out of it.
-                                // Notice that no matter what module cache we get things
-                                // should work


We should preserve this comment, I think.

I do not think so, to be honest.
I added that comment but the way the code is and will be moving that is really a non issue.
We will have a layout cache and types are immutable so the issue is really not there at all

amnn · 2023-05-10T14:38:34Z

crates/sui-core/src/authority.rs

-                                // threading the epoch_store through this API does not
-                                // seem possible, so we just read it from the state (self) and fetch
-                                // the module cache out of it.
-                                // Notice that no matter what module cache we get things
-                                // should work


ditto on preserving the comment here.

amnn · 2023-05-10T14:39:29Z

crates/sui-core/src/authority.rs

-                                // threading the epoch_store through this API does not
-                                // seem possible, so we just read it from the state (self) and fetch
-                                // the module cache out of it.
-                                // Notice that no matter what module cache we get things
-                                // should work


We should preserve this comment, I think.

amnn · 2023-05-10T14:44:43Z

crates/sui-storage/src/package_object_cache.rs

@@ -38,6 +39,17 @@ impl<S: ObjectStore> GetModule for PackageObjectCache<S> {
    }
 }

+impl<S: BackingPackageStore + ModuleResolver<Error = SuiError>> ModuleResolver


I think, technically speaking, you only need to constrain S to implement BackingPackageStore. If it also implemented ModuleResolver<Error = SuiError>, we could implement this as self.store.get_module(id). The reason we don't do that is in the case of TemporaryStore, its BackingPackageStore implementation inspects written_objects looking for packages, while its ModuleResolver implementation does not.

amnn · 2023-05-10T14:56:13Z

crates/sui-types/src/error.rs

@@ -52,6 +52,33 @@ macro_rules! exit_main {
    };
 }

+// {$builder:expr, ($addr:expr)::$module_name:ident::$func:ident($($args:expr),* $(,)?)} => {


This is my bad, but I left this comment here (because I can never remember macro rules' pattern syntax), could you get rid of it?

amnn · 2023-05-10T14:56:30Z

crates/sui-types/src/error.rs

@@ -366,6 +393,8 @@ pub enum SuiError {
    TransactionAlreadyExecuted { digest: TransactionDigest },
    #[error("Object ID did not have the expected type")]
    BadObjectType { error: String },
+    #[error("Fail to retrive Object layout for {st}")]


nit, sp.

Suggested change

#[error("Fail to retrive Object layout for {st}")]

#[error("Fail to retrieve Object layout for {st}")]

amnn · 2023-05-10T15:00:03Z

crates/sui-types/src/temporary_store.rs

+    /// This function is intended to be called *after* we have charged for gas + applied the storage
+    /// rebate to the gas object, but *before* we have updated object versions.  If
+    /// `do_expensive_checks` is false, this function will only check conservation of object storage
+    /// rea `epoch_fees` and `epoch_rebates` are only set for advance epoch transactions.  The


This is my fault -- I just couldn't read it how it was before. I couldn't figure out what "rea" meant before, my guess is "rebates", but I didn't want to make the edit based on a guess, so I left it as it was.

amnn · 2023-05-10T15:02:06Z

crates/transaction-fuzzer/src/account_universe.rs

+            let epoch_store = state.load_epoch_store_one_call_per_task();
+            let move_vm = epoch_store.move_vm();


Let's at least pull this call out of the nested loop, given that it's threateningly named "one_call_per_task".

actually moved everything that I could out of the loop (thanks for pointing out) given that I do not see reasons to keep it in the loop

dariorussi · 2023-05-10T19:11:01Z

made suggested changes unless otherwise commented

- Move expensive_check_sui_conservation to node init - Expose a layout resolver based on a VM session - Decouple MoveResolver traits from Storage traits In some of the places where we want to use the layout resolver, we don't have access to all the information to implement `StorageView`, but we only need to use the `BackingPackageStore`. To support these cases, we need to provide a state view to `LinkageView` that can serve packages, and then shim the Module and Resource resolution functions. This requires that `LinkageView` no longer accept a reference parameter, but a value (so we can inject state into it) -- similar to the trick we pulled to allow `Session` to own a `LinkageView`. Then implementors (like `TemporaryStore`) that only need to be passed by reference can be passed by explicit reference, i.e. `LinkageView<'state, TemporaryStore>` becomes `LinkageView<&'state TemporaryStore>`. However, we cannot impose a `&'state S: StorageView` bound, because some APIs in `StorageView` require a mutable reference, so we needed to decouple the resolution traits (now `SuiResolver`) and the storage traits, to impose twin constraints: ``` fn ...<'state, S> where S: StorageView &'state S: SuiResolver ``` - Logging improvements - Bring `invariant_violation!` macro into `sui-types`. - Create a variant of it -- `make_invariant_violation!` -- that creates the error, but does not wrap it in a result of return it. - Add `format!` string support to these macros - Use this macro for invariant violations within conservation checking with extra context on what is failing (what stage, what type is involved) - Enable telemetry_subscription in the `move_package_upgrade_tests` to see the logs from authorities during tests. - (Temporary) Print the IDs of published/upgraded packages in the failing test, for context.

dariorussi marked this pull request as ready for review April 30, 2023 22:41

dariorussi requested review from lxfind and amnn April 30, 2023 22:41

lxfind reviewed Apr 30, 2023

View reviewed changes

dariorussi force-pushed the upgrade_type3 branch from bce21a0 to 49e0f91 Compare May 1, 2023 00:28

dariorussi commented May 1, 2023

View reviewed changes

dariorussi force-pushed the upgrade_type3 branch from 49e0f91 to 324fdac Compare May 1, 2023 00:38

lxfind requested a review from mystenmark May 1, 2023 00:48

lxfind reviewed May 1, 2023

View reviewed changes

dariorussi force-pushed the upgrade_type3 branch 4 times, most recently from 975ba60 to 0046346 Compare May 2, 2023 15:38

amnn reviewed May 2, 2023

View reviewed changes

lxfind reviewed May 2, 2023

View reviewed changes

dariorussi requested a review from sblackshear May 2, 2023 17:14

dariorussi force-pushed the upgrade_type3 branch 2 times, most recently from bf0470e to 8af9b71 Compare May 3, 2023 12:49

dariorussi force-pushed the upgrade_type3 branch 5 times, most recently from 18a2572 to a0f77ec Compare May 9, 2023 23:07

dariorussi requested a review from tnowacki May 9, 2023 23:40

awelc reviewed May 9, 2023

View reviewed changes

crates/sui-adapter/src/type_layout_resolver.rs Outdated Show resolved Hide resolved

awelc reviewed May 9, 2023

View reviewed changes

crates/sui-adapter/src/type_layout_resolver.rs Show resolved Hide resolved

awelc reviewed May 10, 2023

View reviewed changes

crates/sui-core/src/authority.rs Show resolved Hide resolved

awelc reviewed May 10, 2023

View reviewed changes

dariorussi force-pushed the upgrade_type3 branch 6 times, most recently from c807e84 to 4dce6dc Compare May 10, 2023 13:25

amnn approved these changes May 10, 2023

View reviewed changes

dariorussi force-pushed the upgrade_type3 branch 3 times, most recently from af4afb5 to 2c6d710 Compare May 10, 2023 18:27

dariorussi changed the title ~~Move expensive_check_sui_conservation to node init~~ Create a LayoutResolver to compute layout from VM types May 10, 2023

dariorussi force-pushed the upgrade_type3 branch from 2c6d710 to 70e6a5a Compare May 10, 2023 21:30

dariorussi force-pushed the upgrade_type3 branch from 70e6a5a to db45dd3 Compare May 10, 2023 22:13

dariorussi merged commit 7c6acca into main May 10, 2023

dariorussi deleted the upgrade_type3 branch May 10, 2023 22:42

	// Only initialize an empty database.
	if store
	.database_is_empty()
	.expect("Database read should not fail at init.")
	{

	if let Some(expected_imbalance) = self
	.perpetual_tables
	.expected_storage_fund_imbalance
	.get(&())
	.expect("DB read cannot fail")
	{
	fp_ensure!(
	imbalance == expected_imbalance,
	SuiError::from(
	format!(
	"Inconsistent state detected at epoch {}: total storage rebate: {}, storage fund balance: {}, expected imbalance: {}",
	system_state.epoch, total_storage_rebate, storage_fund_balance, expected_imbalance
	).as_str()
	)
	);
	} else {
	self.perpetual_tables
	.expected_storage_fund_imbalance
	.insert(&(), &imbalance)
	.expect("DB write cannot fail");
	}

	#[error("Fail to retrive Object layout for {st}")]
	#[error("Fail to retrieve Object layout for {st}")]

		let epoch_store = state.load_epoch_store_one_call_per_task();
		let move_vm = epoch_store.move_vm();

Create a LayoutResolver to compute layout from VM types #11529

Create a LayoutResolver to compute layout from VM types #11529

Conversation

dariorussi commented Apr 30, 2023 • edited Loading

Description

Test Plan

Type of Change (Check all that apply)

Release notes

vercel bot commented Apr 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amnn commented Apr 30, 2023

lxfind commented May 1, 2023

lxfind commented May 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariorussi commented May 1, 2023

dariorussi commented May 1, 2023

amnn commented May 1, 2023

dariorussi commented May 1, 2023

lxfind commented May 1, 2023

amnn commented May 1, 2023

lxfind left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariorussi commented May 1, 2023

amnn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariorussi commented May 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awelc May 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariorussi May 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amnn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariorussi commented Apr 30, 2023 •

edited

Loading

vercel bot commented Apr 30, 2023 •

edited

Loading

awelc May 10, 2023 •

edited

Loading

dariorussi May 10, 2023 •

edited

Loading