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

Improve runtime compatibility checking logic. #99

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Neopallium
Copy link

This PR is to improve the substrate-differ to detect more compatibility changes and improve the reported changes.

  1. Expands the compatibility checking to all parts of a pallet: Calls, Storage, Events, Errors, Constants.
  2. Improve type change detection by hashing the type (only the general shape of the type). This allows detecting changes to deeply nested types (i.e. a type that isn't an top-level extrinsic parameter, storage key/value, etc..).
  3. Add a "Require storage migration" check.
  4. Allow enums to add new variants.
  5. If the hash of a type changes, do a direct compatibility check of the old and new types to allow for some changes like new enum variants or wrapping/unwrapping of a type (i.e. OldWeight(pub u64) -> u64)

At Polymesh we added a metadata checker to our CI pipeline to help us spot Breaking API changes:
https://github.com/PolymeshAssociation/Polymesh/tree/develop/metadata-tools
It is using the substrate-differ from this PR.

Here is an example output from one of our PRs. Note the NOT COMPATIBLE that are used to help trace where a breaking change happened. There is also a REQUIRES TX VERSION BUMP marker to help find where a TX breaking change happened, but this report doesn't have a tx API change.

Comparing node spec version 7001000 with stored spec version 7001000
Using stored metadata: .metadata/polymesh_mainnet/7001000.meta
[≠] pallet 0: System -> 1 change(s)
  - constants 1 change(s):
    [≠] OLD: Version: [ 64, 112, 111, 108, 121, 109, 101, 115, 104, 95, 109, 97, 105, 110, 110, 101, 116, 64, 112, 111, 108, 121, 109, 101, 115, 104, 95, 109, 97, 105, 110, 110, ... ]
        NEW: Version: [ 64, 112, 111, 108, 121, 109, 101, 115, 104, 95, 109, 97, 105, 110, 110, 101, 116, 64, 112, 111, 108, 121, 109, 101, 115, 104, 95, 109, 97, 105, 110, 110, ... ]
        CHANGES: Value changed

[≠] pallet 1: Babe -> 4 change(s) NOT COMPATIBLE
  - calls 2 change(s):
    [≠] OLD:  0: report_equivocation ( equivocation_proof: EquivocationProof<Header<u32, BlakeTwo256>, Public>, key_owner_proof: MembershipProof )
        NEW:  0: report_equivocation ( equivocation_proof: EquivocationProof<Header<u32, Hash>, Public>, key_owner_proof: MembershipProof )
        CHANGES: Sig Changed: [≠] 0: Type changed: Name changed: EquivocationProof<Header<u32, BlakeTwo256>, Public> -> EquivocationProof<Header<u32, Hash>, Public>
    [≠] OLD:  1: report_equivocation_unsigned ( equivocation_proof: EquivocationProof<Header<u32, BlakeTwo256>, Public>, key_owner_proof: MembershipProof )
        NEW:  1: report_equivocation_unsigned ( equivocation_proof: EquivocationProof<Header<u32, Hash>, Public>, key_owner_proof: MembershipProof )
        CHANGES: Sig Changed: [≠] 0: Type changed: Name changed: EquivocationProof<Header<u32, BlakeTwo256>, Public> -> EquivocationProof<Header<u32, Hash>, Public>

  - constants 1 change(s):
    [+] MaxNominators: [0, 4, 0, 0]

  - storages 1 change(s): NOT COMPATIBLE
    [≠] OLD: Initialized: Option<PreDigest> (Optional [0])
        NEW: Initialized: Option<PreDigest> (Optional [0])
        NOT COMPATIBLE
        CHANGES: Type changed: Plain: Type changed: Option<PreDigest> -> PreDigest -> PrimaryPreDigest -> Different number of fields

[≠] pallet 7: Identity -> 1 change(s)
  - calls 1 change(s):
    [≠] OLD: 11: remove_authorization ( target: Signatory<AccountId32>, auth_id: u64, _auth_issuer_pays: bool )
        NEW: 11: remove_authorization ( target: Signatory<AccountId32>, auth_id: u64, auth_issuer_pays: bool )
        CHANGES: Sig Changed: [≠] 2: Name changed: _auth_issuer_pays -> auth_issuer_pays

[≠] pallet 8: CddServiceProviders -> 1 change(s) NOT COMPATIBLE
storage_prefix: StringChange("Instance2Group", "CddServiceProviders")

[≠] pallet 9: PolymeshCommittee -> 1 change(s) NOT COMPATIBLE
storage_prefix: StringChange("Instance1Committee", "PolymeshCommittee")

[≠] pallet 10: CommitteeMembership -> 1 change(s) NOT COMPATIBLE
storage_prefix: StringChange("Instance1Group", "CommitteeMembership")

[≠] pallet 11: TechnicalCommittee -> 1 change(s) NOT COMPATIBLE
storage_prefix: StringChange("Instance3Committee", "TechnicalCommittee")

[≠] pallet 12: TechnicalCommitteeMembership -> 1 change(s) NOT COMPATIBLE
storage_prefix: StringChange("Instance3Group", "TechnicalCommitteeMembership")

[≠] pallet 13: UpgradeCommittee -> 1 change(s) NOT COMPATIBLE
storage_prefix: StringChange("Instance4Committee", "UpgradeCommittee")

[≠] pallet 14: UpgradeCommitteeMembership -> 1 change(s) NOT COMPATIBLE
storage_prefix: StringChange("Instance4Group", "UpgradeCommitteeMembership")

[≠] pallet 18: Offences -> 1 change(s) NOT COMPATIBLE
  - storages 1 change(s): NOT COMPATIBLE
    [-] ReportsByKindIndex

[≠] pallet 21: Grandpa -> 1 change(s)
  - constants 1 change(s):
    [+] MaxNominators: [0, 4, 0, 0]

[≠] pallet 23: ImOnline -> 2 change(s) NOT COMPATIBLE
  - calls 1 change(s): NOT COMPATIBLE
    [≠] OLD:  0: heartbeat ( heartbeat: Heartbeat<u32>, signature: Signature )
        NEW:  0: heartbeat ( heartbeat: Heartbeat<u32>, signature: Signature )
        NOT COMPATIBLE
        CHANGES: Sig Changed: [≠] 0: Type changed: Type changed: Heartbeat<u32> -> Different number of fields

  - storages 1 change(s): NOT COMPATIBLE
    [≠] OLD: ReceivedHeartbeats: (map<twox_64_concat, twox_64_concat>: key: (u32, u32) => value: WrapperOpaque<BoundedOpaqueNetworkState<PeerIdEncodingLimit, MultiAddrEncodingLimit, AddressesLimit>>) (Optional [0])
        NEW: ReceivedHeartbeats: (map<twox_64_concat, twox_64_concat>: key: (u32, u32) => value: bool) (Optional [0])
        NOT COMPATIBLE
        CHANGES: Type changed: Value changed: Type changed: WrapperOpaque<BoundedOpaqueNetworkState<PeerIdEncodingLimit, MultiAddrEncodingLimit, AddressesLimit>> -> Different type definition

[≠] pallet 34: Portfolio -> 2 change(s)
  - constants 2 change(s):
    [+] MaxNumberOfFungibleMoves: [10, 0, 0, 0]
    [+] MaxNumberOfNFTsMoves: [100, 0, 0, 0]

[≠] pallet 37: Settlement -> 1 change(s)
  - constants 1 change(s):
    [+] MaxInstructionMediators: [4, 0, 0, 0]

[≠] pallet 46: Contracts -> 35 change(s) NOT COMPATIBLE
  - calls 4 change(s):
    [≠] OLD:  0: call_old_weight ( dest: MultiAddress<AccountId32, u32>, value: Compact<u128>, gas_limit: Compact<OldWeight>, storage_deposit_limit: Option<Compact<u128>>, data: Vec<u8> )
        NEW:  0: call_old_weight ( dest: MultiAddress<AccountId32, u32>, value: Compact<u128>, gas_limit: Compact<u64>, storage_deposit_limit: Option<Compact<u128>>, data: Vec<u8> )
        CHANGES: Sig Changed: [≠] 2: Type changed: Name changed: Compact<OldWeight> -> Compact<u64>
    [≠] OLD:  1: instantiate_with_code_old_weight ( value: Compact<u128>, gas_limit: Compact<OldWeight>, storage_deposit_limit: Option<Compact<u128>>, code: Vec<u8>, data: Vec<u8>, salt: Vec<u8> )
        NEW:  1: instantiate_with_code_old_weight ( value: Compact<u128>, gas_limit: Compact<u64>, storage_deposit_limit: Option<Compact<u128>>, code: Vec<u8>, data: Vec<u8>, salt: Vec<u8> )
        CHANGES: Sig Changed: [≠] 1: Type changed: Name changed: Compact<OldWeight> -> Compact<u64>
    [≠] OLD:  2: instantiate_old_weight ( value: Compact<u128>, gas_limit: Compact<OldWeight>, storage_deposit_limit: Option<Compact<u128>>, code_hash: H256, data: Vec<u8>, salt: Vec<u8> )
        NEW:  2: instantiate_old_weight ( value: Compact<u128>, gas_limit: Compact<u64>, storage_deposit_limit: Option<Compact<u128>>, code_hash: H256, data: Vec<u8>, salt: Vec<u8> )
        CHANGES: Sig Changed: [≠] 1: Type changed: Name changed: Compact<OldWeight> -> Compact<u64>
    [+]  9: migrate ( weight_limit: Weight )

  - events 1 change(s): NOT COMPATIBLE
    [≠] OLD:  6: Called ( caller: AccountId32, contract: AccountId32 )
        NEW:  6: Called ( caller: Origin<Runtime>, contract: AccountId32 )
        NOT COMPATIBLE
        CHANGES: Sig Changed: [≠] 0: Type changed: Type changed: AccountId32 -> Different type definition

  - errors 17 change(s):
    [≠] OLD:  0: InvalidScheduleVersion
        NEW:  0: InvalidSchedule 
        CHANGES: Name(StringChange("InvalidScheduleVersion", "InvalidSchedule"))
    [≠] OLD:  9: OutOfBounds     
        NEW:  9: CodeInfoNotFound
        CHANGES: Name(StringChange("OutOfBounds", "CodeInfoNotFound"))
    [≠] OLD: 10: DecodingFailed  
        NEW: 10: OutOfBounds     
        CHANGES: Name(StringChange("DecodingFailed", "OutOfBounds"))
    [≠] OLD: 11: ContractTrapped 
        NEW: 11: DecodingFailed  
        CHANGES: Name(StringChange("ContractTrapped", "DecodingFailed"))
    [≠] OLD: 12: ValueTooLarge   
        NEW: 12: ContractTrapped 
        CHANGES: Name(StringChange("ValueTooLarge", "ContractTrapped"))
    [≠] OLD: 13: TerminatedWhileReentrant
        NEW: 13: ValueTooLarge   
        CHANGES: Name(StringChange("TerminatedWhileReentrant", "ValueTooLarge"))
    [≠] OLD: 14: InputForwarded  
        NEW: 14: TerminatedWhileReentrant
        CHANGES: Name(StringChange("InputForwarded", "TerminatedWhileReentrant"))
    [≠] OLD: 15: RandomSubjectTooLong
        NEW: 15: InputForwarded  
        CHANGES: Name(StringChange("RandomSubjectTooLong", "InputForwarded"))
    [≠] OLD: 16: TooManyTopics   
        NEW: 16: RandomSubjectTooLong
        CHANGES: Name(StringChange("TooManyTopics", "RandomSubjectTooLong"))
    [≠] OLD: 17: NoChainExtension
        NEW: 17: TooManyTopics   
        CHANGES: Name(StringChange("NoChainExtension", "TooManyTopics"))
    [≠] OLD: 18: DeletionQueueFull
        NEW: 18: NoChainExtension
        CHANGES: Name(StringChange("DeletionQueueFull", "NoChainExtension"))
    [+] 28: MigrationInProgress
    [+] 29: NoMigrationPerformed
    [+] 30: MaxDelegateDependenciesReached
    [+] 31: DelegateDependencyNotFound
    [+] 32: DelegateDependencyAlreadyExists
    [+] 33: CannotAddSelfAsDelegateDependency

  - constants 6 change(s): NOT COMPATIBLE
    [+] CodeHashLockupDepositPercent: [0, 163, 225, 17]
    [+] DefaultDepositLimit: [0, 192, 39, 175, 14, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
    [+] MaxDelegateDependencies: [32, 0, 0, 0]
    [≠] OLD: Schedule: [ 4, 0, 0, 0, 0, 1, 0, 0, 0, 4, 0, 0, 128, 0, 0, 0, 16, 0, 0, 0, 0, 16, 0, 0, 0, 1, 0, 0, 32, 0, 0, 0, ... ]
        NEW: Schedule: [ 4, 0, 0, 0, 0, 1, 0, 0, 0, 4, 0, 0, 128, 0, 0, 0, 16, 0, 0, 0, 0, 16, 0, 0, 0, 1, 0, 0, 32, 0, 0, 0, ... ]
        NOT COMPATIBLE
        CHANGES: Value changed
        CHANGES: Type changed: Type changed: Schedule<T> -> Limits -> Different number of fields
    [-] DeletionQueueDepth
    [-] DeletionWeightLimit

  - storages 7 change(s): NOT COMPATIBLE
    [+] CodeInfoOf: (map<identity>: key: H256 => value: CodeInfo<T>) (Optional [0])
    [≠] OLD: ContractInfoOf: (map<twox_64_concat>: key: AccountId32 => value: ContractInfo<T>) (Optional [0])
        NEW: ContractInfoOf: (map<twox_64_concat>: key: AccountId32 => value: ContractInfo<T>) (Optional [0])
        NOT COMPATIBLE
        CHANGES: Type changed: Value changed: Type changed: ContractInfo<T> -> Different number of fields
    [≠] OLD: DeletionQueue: BoundedVec<DeletedContract, S> (Default  [0])
        NEW: DeletionQueue: (map<twox_64_concat>: key: u32 => value: BoundedVec<u8, S>) (Optional [0])
        NOT COMPATIBLE
        CHANGES: Modifier changed: Default -> Optional
        CHANGES: Type changed: Old: BoundedVec<DeletedContract, S>, New: (map<twox_64_concat>: key: u32 => value: BoundedVec<u8, S>)
    [+] DeletionQueueCounter: DeletionQueueManager<T> (Default  [0; 8])
    [+] MigrationInProgress: BoundedVec<u8, S> (Optional [0])
    [-] CodeStorage
    [-] OwnerInfoOf

[≠] pallet 49: Nft -> 1 change(s) NOT COMPATIBLE
storage_prefix: StringChange("NFT", "Nft")

[≠] pallet 50: ElectionProviderMultiPhase -> 3 change(s)
  - constants 1 change(s):
    [+] MinerMaxWinners: [232, 3, 0, 0]

  - storages 2 change(s):
    [≠] OLD: QueuedSolution: ReadySolution<T> (Optional [0])
        NEW: QueuedSolution: ReadySolution<AccountId, MaxWinners> (Optional [0])
        CHANGES: Type changed: Plain: Name changed: ReadySolution<T> -> ReadySolution<AccountId, MaxWinners>
    [≠] OLD: Snapshot: RoundSnapshot<T> (Optional [0])
        NEW: Snapshot: RoundSnapshot<AccountId32, (AccountId32, u64, BoundedVec<AccountId32, S>)> (Optional [0])
        CHANGES: Type changed: Plain: Name changed: RoundSnapshot<T> -> RoundSnapshot<AccountId32, (AccountId32, u64, BoundedVec<AccountId32, S>)>

SUMMARY:
- Compatible.......................: false
- Require transaction_version bump.: false
- Require storage migration........: true

@chevdor
Copy link
Owner

chevdor commented Jan 16, 2025

Very nice, thanks for the PR @Neopallium and happy to see you found this crate helpful.
Would you mind updating the crates' readme in libs/substrae-differ/README.md to reflect your changes ?
Feel free to drop a note in the main readme if you wish, but there you have to do it in README_src.adoc and not in the README.md (which is generated).

I would also love to have a few tests showing at least 2 scenarri where "requires storage migration" return true and false.
You can use any 2/3 of your runtimes for that. It makes it easier if you add the commands to fetch them in fetch-wasm.sh for instance.

Those tests will NOT run in CI if you flag them as local-data, yet they can be ran manually:

#[ignore = "local data"]
	fn test_....() {
		// ...
	}

Those local data are no ran via CI because I don't want to start trying to fetch ancient runtimes but it is good to have them to ensure from time to time that everything still runs fine.

I will miss an update of the doc for subwasm itself but I can take care of generating a new one once the PR is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants