Skip to content

Commit

Permalink
feat: Use scrypto_decode_with_nice_error and backwards compatability (
Browse files Browse the repository at this point in the history
#1010)

## Summary

(Blocked on #1008 for
merging into `develop`)

* Uses `scrypto_decode_with_nice_error` in a few places where a
mis-decode will result in a panic... To help us debug if something bad
goes wrong.
* Adds `CheckedBackwardsCompatibleSchema` requirements and assertions to
all SBOR-based field codecs.

## Testing

Existing and new tests pass
  • Loading branch information
dhedey authored Oct 31, 2024
2 parents 9f66bc9 + f138bf2 commit ae2ed91
Show file tree
Hide file tree
Showing 31 changed files with 207 additions and 80 deletions.
38 changes: 19 additions & 19 deletions core-rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 12 additions & 12 deletions core-rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,18 @@ resolver = "2"
# Then use tag="release_name-BLAH" in the below dependencies.
# =================================================================

sbor = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023", features = ["serde"] }
radix-transactions = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023" }
radix-transaction-scenarios = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023" }
radix-common = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023", features = ["serde"] }
radix-engine-interface = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023" }
radix-engine = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023" }
radix-substate-store-impls = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023" }
radix-substate-store-interface = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023" }
radix-substate-store-queries = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023" }
radix-rust = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023", features = ["serde"] }
radix-blueprint-schema-init = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023", features = ["serde"] }
radix-engine-toolkit-common = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-28bbc023" }
sbor = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405", features = ["serde"] }
radix-transactions = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405" }
radix-transaction-scenarios = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405" }
radix-common = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405", features = ["serde"] }
radix-engine-interface = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405" }
radix-engine = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405" }
radix-substate-store-impls = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405" }
radix-substate-store-interface = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405" }
radix-substate-store-queries = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405" }
radix-rust = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405", features = ["serde"] }
radix-blueprint-schema-init = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405", features = ["serde"] }
radix-engine-toolkit-common = { git = "https://github.com/radixdlt/radixdlt-scrypto", tag = "cuttlefish-a583a405" }

itertools = { version = "=0.10.5" }
jni = { version = "=0.19.0" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ pub struct MetadataKey {
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum AttachedModuleBrowsingError {
UnderlyingError(EngineStateBrowsingError),
SborDecode(DecodeError),
SborDecode(String),
NotAnObject,
AttachedModulesInvariantBroken(String),
}
Expand Down Expand Up @@ -355,24 +355,23 @@ impl From<AttachedModuleBrowsingError> for ResponseError {
}
}

fn decode_kv_collection_key<D: ScryptoDecode>(key: SborCollectionKey) -> D {
fn decode_kv_collection_key<D: ScryptoDecode + ScryptoDescribe>(key: SborCollectionKey) -> D {
let SborCollectionKey::KeyValueStore(sbor_data) = key else {
panic!("expected the Key-Value collection key; got {:?}", key)
};
let Ok(decoded) = scrypto_decode(sbor_data.as_bytes()) else {
scrypto_decode_with_nice_error(sbor_data.as_bytes()).unwrap_or_else(|err| {
panic!(
"expected the collection key to be a {}; got {:?}",
type_name::<D>(),
sbor_data
err,
)
};
decoded
})
}

fn decode_latest<V: Versioned + ScryptoDecode>(
fn decode_latest<V: Versioned + ScryptoDecode + ScryptoDescribe>(
entry_data: SborData,
) -> Result<V::LatestVersion, AttachedModuleBrowsingError> {
Ok(scrypto_decode::<V>(entry_data.as_bytes())
Ok(scrypto_decode_with_nice_error::<V>(entry_data.as_bytes())
.map_err(AttachedModuleBrowsingError::SborDecode)?
.fully_update_and_into_latest_version())
}
Expand Down
39 changes: 32 additions & 7 deletions core-rust/node-common/src/store/typed_cf_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ impl<
/// encoding is used for values.
pub trait VersionedCf {
type Key;
type Value;
type Value: Into<Self::VersionedValue> + Clone;

/// Column family name (as known to the DB).
///
Expand All @@ -539,13 +539,21 @@ pub trait VersionedCf {
/// Key codec type.
type KeyCodec: Default;
/// Versioned **value** type (a codec for it is known, i.e. SBOR-based).
type VersionedValue;
type VersionedValue: ScryptoEncode
+ ScryptoDecode
+ ScryptoDescribe
+ CheckedBackwardsCompatibleSchema<ScryptoCustomSchema>
+ Versioned<LatestVersion = Self::Value>;
}

impl<K, V, VV, KC, D> DefaultCf for D
where
V: Into<VV> + Clone,
VV: ScryptoEncode + ScryptoDecode + Versioned<LatestVersion = V>,
VV: ScryptoEncode
+ ScryptoDecode
+ ScryptoDescribe
+ CheckedBackwardsCompatibleSchema<ScryptoCustomSchema>
+ Versioned<LatestVersion = V>,
KC: Default,
D: VersionedCf<Key = K, Value = V, KeyCodec = KC, VersionedValue = VV>,
{
Expand Down Expand Up @@ -692,25 +700,42 @@ impl<U: DbCodec<VT>, T: Into<VT> + Clone, VT: Versioned<LatestVersion = T>> DbCo
}

/// A [`DbCodec]` for SBOR types.
pub struct SborDbCodec<T: ScryptoEncode + ScryptoDecode> {
pub struct SborDbCodec<
T: ScryptoEncode
+ ScryptoDecode
+ ScryptoDescribe
+ CheckedBackwardsCompatibleSchema<ScryptoCustomSchema>,
> {
type_parameters_phantom: PhantomData<T>,
}

impl<T: ScryptoEncode + ScryptoDecode> Default for SborDbCodec<T> {
impl<
T: ScryptoEncode
+ ScryptoDecode
+ ScryptoDescribe
+ CheckedBackwardsCompatibleSchema<ScryptoCustomSchema>,
> Default for SborDbCodec<T>
{
fn default() -> Self {
Self {
type_parameters_phantom: PhantomData,
}
}
}

impl<T: ScryptoEncode + ScryptoDecode> DbCodec<T> for SborDbCodec<T> {
impl<
T: ScryptoEncode
+ ScryptoDecode
+ ScryptoDescribe
+ CheckedBackwardsCompatibleSchema<ScryptoCustomSchema>,
> DbCodec<T> for SborDbCodec<T>
{
fn encode(&self, value: &T) -> Vec<u8> {
scrypto_encode(value).unwrap()
}

fn decode(&self, bytes: &[u8]) -> T {
scrypto_decode(bytes).unwrap()
scrypto_decode_with_nice_error(bytes).unwrap()
}
}

Expand Down
Binary file added core-rust/p2p/src/CF_SCHEMA_migration_status.bin
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
16 changes: 14 additions & 2 deletions core-rust/p2p/src/address_book_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,25 @@ pub struct AddressBookEntryV1 {

define_single_versioned! {
#[derive(Debug, Clone, ScryptoSbor)]
pub VersionedAddressBookEntry(AddressBookEntryVersions) => AddressBookEntry = AddressBookEntryV1
pub VersionedAddressBookEntry(AddressBookEntryVersions) => AddressBookEntry = AddressBookEntryV1,
outer_attributes: [
#[derive(ScryptoSborAssertion)]
#[sbor_assert(backwards_compatible(
cuttlefish = "FILE:CF_SCHEMA_versioned_address_book_entry.bin"
))]
]
}

#[derive(Debug, Clone, ScryptoSbor)]
pub struct HighPriorityPeersV1(Vec<NodeIdDTO>);

define_single_versioned! {
#[derive(Debug, Clone, ScryptoSbor)]
pub VersionedHighPriorityPeers(HighPriorityPeersVersions) => HighPriorityPeers = HighPriorityPeersV1
pub VersionedHighPriorityPeers(HighPriorityPeersVersions) => HighPriorityPeers = HighPriorityPeersV1,
outer_attributes: [
#[derive(ScryptoSborAssertion)]
#[sbor_assert(backwards_compatible(
cuttlefish = "FILE:CF_SCHEMA_versioned_high_priority_peers.bin"
))]
]
}
5 changes: 3 additions & 2 deletions core-rust/p2p/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@
* permissions under this License.
*/

use sbor::Sbor;
use radix_common::prelude::*;

/// Status of the migration
#[repr(u8)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Sbor)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Sbor, ScryptoSborAssertion)]
#[sbor_assert(backwards_compatible(cuttlefish = "FILE:CF_SCHEMA_migration_status.bin"))]
pub enum MigrationStatus {
Completed,
}
8 changes: 7 additions & 1 deletion core-rust/p2p/src/safety_store_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,13 @@ pub struct SafetyStateV1 {

define_single_versioned! {
#[derive(Debug, Clone, ScryptoSbor)]
pub VersionedSafetyState(SafetyStateVersions) => SafetyState = SafetyStateV1
pub VersionedSafetyState(SafetyStateVersions) => SafetyState = SafetyStateV1,
outer_attributes: [
#[derive(ScryptoSborAssertion)]
#[sbor_assert(backwards_compatible(
cuttlefish = "FILE:CF_SCHEMA_versioned_safety_state.bin"
))]
]
}

/// Safety state components. Note that these structs are intended only for proper encoding/deconding
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ impl ProtocolUpdateDefinition for BabylonProtocolUpdateDefinition {
Some(overrides) => overrides,
None => {
let raw_genesis_data = context.genesis_data_resolver.get_raw_genesis_data();
scrypto_decode(&raw_genesis_data).expect("Could not decode genesis data")
scrypto_decode_with_nice_error(&raw_genesis_data)
.expect("Could not decode genesis data")
}
};

Expand Down
Loading

0 comments on commit ae2ed91

Please sign in to comment.