Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

chore: v0.9.38 #194

Merged
merged 40 commits into from
May 22, 2023
Merged

chore: v0.9.38 #194

merged 40 commits into from
May 22, 2023

Conversation

enthusiastmartin
Copy link
Contributor

@enthusiastmartin enthusiastmartin commented Mar 30, 2023

based on #174.

Notable changes:

  • required to install protobuf compiler
    sudo apt-get install protobuf-compiler
  • for math repo, required to install m4
    sudo apt-get install m4
  • pallet stuff renamed
    • Event → RuntimeEvent
    • From + IsType<::RuntimeEvent>;
    • Origin → RuntimeOrigin
    • Call → RuntimeCall
  • extrinsics need call_index ( otherwise depreated warning )
    #[pallet::call_index(0)]
  • orml traits
    OnDust trait move into submodule called currency
  • NFT pallet
    InspectEnumerable trait changed - needs closer look to verify f reimplemented correcy.
  • pallet currencies
    check if changes in orml currencies to ensure no bugs fixed ?
  • orml tokens
    callbacks OnDust,OnNewAccoutn and OnKilledAccount combined into one CurrencyHooks.

@codecov
Copy link

codecov bot commented Apr 20, 2023

Codecov Report

Patch coverage: 84.61% and project coverage change: -0.06 ⚠️

Comparison is base (b928c0c) 73.96% compared to head (6649bf4) 73.91%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #194      +/-   ##
==========================================
- Coverage   73.96%   73.91%   -0.06%     
==========================================
  Files          45       45              
  Lines        3607     3615       +8     
==========================================
+ Hits         2668     2672       +4     
- Misses        939      943       +4     
Impacted Files Coverage Δ
adapters/src/inspect.rs 14.81% <0.00%> (-2.58%) ⬇️
asset-registry/src/lib.rs 81.63% <ø> (ø)
collator-rewards/src/lib.rs 80.95% <ø> (ø)
currencies/src/lib.rs 62.40% <ø> (ø)
duster/src/lib.rs 60.81% <ø> (ø)
duster/src/migration.rs 0.00% <ø> (ø)
ema-oracle/src/lib.rs 86.57% <ø> (ø)
faucet/src/lib.rs 82.35% <ø> (ø)
liquidity-mining/src/lib.rs 75.94% <ø> (ø)
otc/src/lib.rs 90.00% <ø> (ø)
... and 13 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Roznovjak Roznovjak mentioned this pull request Apr 22, 2023
@Roznovjak Roznovjak marked this pull request as ready for review April 26, 2023 14:40
Copy link
Collaborator

@apopiak apopiak left a comment

Choose a reason for hiding this comment

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

LGTM

@Roznovjak Roznovjak changed the base branch from v0.9.37 to main May 3, 2023 14:54
@apopiak
Copy link
Collaborator

apopiak commented May 18, 2023

we should make sure to include #206 (just merged)

adapters/src/inspect.rs Outdated Show resolved Hide resolved
@@ -19,7 +19,6 @@ use crate::Config;
use frame_support::weights::Weight;
use sp_std::vec::Vec;

/// Storage names are changed from Classes to Collections and from Instances to Items.
pub mod v1 {
Copy link
Member

Choose a reason for hiding this comment

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

do we still need this migration at all?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, based on the storage version in Basilisk and Hydra, we will need it.

@@ -233,6 +237,7 @@ pub mod pallet {
/// Parameters:
/// - `origin`: The collection owner.
/// - `collection_id`: The identifier of the asset collection to be destroyed.
#[pallet::call_index(4)]
Copy link
Member

Choose a reason for hiding this comment

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

Did you check that the call index is set to the same index as it was before? it seems like it does out by my random checks, but I didn't check every single one. seems like pattern is ok

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, but based on my assumption and this comment indexes are assigned sequentially and in order.

@@ -75,7 +75,7 @@ fn create_collection_works() {
// reserved collection ID
assert_noop!(
NFTPallet::create_collection(
Origin::signed(ALICE),
RuntimeOrigin::signed(ALICE),
Copy link
Member

Choose a reason for hiding this comment

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

Why was Origin renamed to RuntimeOrigin and Event to RuntimeEvent?

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMHO to differentiate outer enums that are created by construct_runtime macro from nested enums from pallets.
You can see it in this example:
RuntimeCall::Balances(pallet_balances::Call::transfer { dest: ALICE, value: 10 })

test-utils/src/rpc_service.rs Outdated Show resolved Hide resolved
transaction-pause/src/tests.rs Outdated Show resolved Hide resolved
@mrq1911 mrq1911 merged commit f64f502 into main May 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants