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

Feature: Versioned package substates #1358

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

dhedey
Copy link
Contributor

@dhedey dhedey commented Aug 21, 2023

Summary

A slightly stripped back #1318 which avoids generating the _StateInit and the _Partition models.

I've generally tried to avoid changing things that don't need changing, but I don't think there's anything particularly controversial left?

This implements for packages, and in a separate PR, I'll implement for other native blueprints.

Update Recommendations

For Internal Integrators

I'll merge this into the node. This shouldn't have any impact outside of Scrypto and Node.

@github-actions
Copy link

github-actions bot commented Aug 21, 2023

Benchmark for a3f2a35

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 64.7±0.50ms 68.6±1.15ms +6.03%
costing::decode_sbor 16.9±0.02µs 16.9±0.04µs 0.00%
costing::deserialize_wasm 1433.3±5.10µs 1441.7±2.61µs +0.59%
costing::instantiate_flash_loan 5.8±0.02ms 6.0±0.03ms +3.45%
costing::instantiate_radiswap 5.3±0.02ms 5.9±0.06ms +11.32%
costing::spin_loop 8.4±0.02ms 8.5±0.09ms +1.19%
costing::validate_sbor_payload 29.9±0.04µs 30.1±0.17µs +0.67%
costing::validate_secp256k1 84.6±0.14µs 84.7±0.22µs +0.12%
costing::validate_wasm 35.5±0.05ms 35.6±0.15ms +0.28%
decimal::add/0 7.2±0.01ns 7.2±0.00ns 0.00%
decimal::add/rust-native 9.5±0.01ns 9.5±0.02ns 0.00%
decimal::add/wasmer 143.7±0.24ns 144.2±0.15ns +0.35%
decimal::add/wasmer-call-native 616.8±0.74ns 593.6±0.76ns -3.76%
decimal::add/wasmi 487.2±4.06ns 480.7±1.41ns -1.33%
decimal::add/wasmi-call-native 6.0±0.04µs 6.0±0.02µs 0.00%
decimal::div/0 153.7±0.15ns 154.8±0.15ns +0.72%
decimal::from_string/0 192.6±0.24ns 193.1±0.20ns +0.26%
decimal::mul/0 109.7±0.05ns 109.9±0.23ns +0.18%
decimal::mul/rust-native 113.2±0.16ns 113.7±0.09ns +0.44%
decimal::mul/wasmer 1693.0±1.15ns 1696.0±2.78ns +0.18%
decimal::mul/wasmer-call-native 731.9±0.24ns 707.1±0.34ns -3.39%
decimal::mul/wasmi 29.3±0.05µs 29.7±0.43µs +1.37%
decimal::mul/wasmi-call-native 6.3±0.04µs 6.1±0.03µs -3.17%
decimal::pow/0 530.4±0.14ns 550.6±18.78ns +3.81%
decimal::pow/rust-native 532.6±0.18ns 533.9±1.11ns +0.24%
decimal::pow/wasmer 7.5±0.01µs 7.5±0.01µs 0.00%
decimal::pow/wasmer-call-native 1191.4±0.40ns 1177.8±2.05ns -1.14%
decimal::pow/wasmi 140.1±0.69µs 139.6±0.26µs -0.36%
decimal::pow/wasmi-call-native 8.5±0.08µs 8.5±0.09µs 0.00%
decimal::root/0 9.6±0.02µs 9.5±0.01µs -1.04%
decimal::sub/0 7.2±0.00ns 7.2±0.01ns 0.00%
decimal::to_string/0 513.9±0.45ns 532.8±2.66ns +3.68%
precise_decimal::add/0 7.8±0.00ns 7.8±0.02ns 0.00%
precise_decimal::add/rust-native 9.6±0.01ns 9.6±0.12ns 0.00%
precise_decimal::add/wasmer 148.9±0.12ns 149.5±0.04ns +0.40%
precise_decimal::add/wasmer-call-native 601.8±0.54ns 605.0±0.59ns +0.53%
precise_decimal::add/wasmi 592.2±0.43ns 591.6±8.34ns -0.10%
precise_decimal::add/wasmi-call-native 6.5±0.06µs 6.5±0.04µs 0.00%
precise_decimal::div/0 278.0±0.10ns 278.5±0.25ns +0.18%
precise_decimal::from_string/0 275.2±0.75ns 275.4±1.22ns +0.07%
precise_decimal::mul/0 307.8±0.18ns 307.9±0.14ns +0.03%
precise_decimal::mul/rust-native 276.0±0.85ns 278.8±0.14ns +1.01%
precise_decimal::mul/wasmer 4.0±0.00µs 4.1±0.01µs +2.50%
precise_decimal::mul/wasmer-call-native 920.5±42.07ns 907.3±1.30ns -1.43%
precise_decimal::mul/wasmi 84.9±12.69µs 80.1±0.05µs -5.65%
precise_decimal::mul/wasmi-call-native 6.9±0.05µs 6.8±0.05µs -1.45%
precise_decimal::pow/0 1658.9±3.15ns 1641.4±0.16ns -1.05%
precise_decimal::pow/rust-native 1302.2±1.11ns 1295.6±0.57ns -0.51%
precise_decimal::pow/wasmer 19.2±0.70µs 19.0±0.02µs -1.04%
precise_decimal::pow/wasmer-call-native 2.1±0.00µs 2.1±0.01µs 0.00%
precise_decimal::pow/wasmi 388.2±0.52µs 387.3±0.40µs -0.23%
precise_decimal::pow/wasmi-call-native 11.2±0.06µs 11.2±0.03µs 0.00%
precise_decimal::root/0 62.4±0.04µs 62.5±0.05µs +0.16%
precise_decimal::sub/0 8.3±0.01ns 8.3±0.01ns 0.00%
precise_decimal::to_string/0 753.5±0.63ns 770.9±0.36ns +2.31%
schema::validate_payload 255.9±1.60µs 253.9±0.87µs -0.78%
transaction::radiswap 5.4±0.01ms 5.4±0.08ms 0.00%
transaction::transfer 2.2±0.01ms 2.2±0.00ms 0.00%
transaction_processing::prepare 2.6±0.10ms 2.7±0.00ms +3.85%
transaction_processing::prepare_and_decompile 6.7±0.05ms 6.7±0.05ms 0.00%
transaction_processing::prepare_and_decompile_and_recompile 23.4±0.06ms 24.0±0.22ms +2.56%
transaction_validation::validate_manifest 46.2±0.05µs 46.3±0.36µs +0.22%
transaction_validation::verify_ecdsa 82.2±0.07µs 82.2±0.17µs 0.00%
transaction_validation::verify_ed25519 51.6±0.09µs 51.8±0.07µs +0.39%

OriginalCode,
InstrumentedCode,
}
macro_rules! blueprint_partition_offset {
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove these mappings as discussed?

Copy link
Contributor Author

@dhedey dhedey Aug 21, 2023

Choose a reason for hiding this comment

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

Not easily - these are currently used by the TypedSubstate mapping, which we need for reverse-mapping the substate stream to typed substates.

@@ -0,0 +1,157 @@
use crate::internal_prelude::*;

pub trait FeatureSetResolver: Debug {
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename to something like HasFeatures, Resolver seems to imply a process which takes input.


/// For feature checks against a non-inner object
#[derive(Debug)]
pub enum FeatureChecks<TOwn: FeatureSetResolver> {
Copy link
Member

Choose a reason for hiding this comment

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

As discussed can we remove FeatureChecks?


/// Generates types and typed-interfaces for native blueprints, their
/// state models, features, partitions, and their interaction with
/// the substate store.
Copy link
Member

Choose a reason for hiding this comment

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

Update comment to remove "partitions and their interactions with the substate store"

/// For each collection key, the following types will be created:
/// * `<BlueprintIdent><CollectionIdent>KeyPayload` - a new type for the key payload (eg includes the u16 for a sorted index key)
///
/// The content of each of the above can take a number of forms.
Copy link
Member

Choose a reason for hiding this comment

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

Except for keys right?

blueprint_version_definitions: KeyValue {
entry_ident: BlueprintVersionDefinition,
key_type: {
kind: Static,
Copy link
Member

Choose a reason for hiding this comment

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

Does key_type have another kind besides Static?

@dhedey dhedey force-pushed the feature/core-substate-model-versioning branch from a10d0ff to 4518cf8 Compare August 21, 2023 15:34
@dhedey dhedey merged commit a0b8ddc into develop Aug 21, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants