Skip to content

Conversation

@aurexav
Copy link
Contributor

@aurexav aurexav commented Nov 12, 2024

Make pallet-recovery supports BlockNumberProvider.

Part of #6297.


Polkadot address: 156HGo9setPcU2qhFMVWLkcmtCEGySLwNqa3DaEiYSWtte4Y

@aurexav aurexav marked this pull request as ready for review November 15, 2024 07:58
@aurexav aurexav force-pushed the blocknumberprovider-recovery branch from 93b255d to 976443e Compare November 15, 2024 07:58
@paritytech-review-bot paritytech-review-bot bot requested a review from a team November 15, 2024 07:59
@gui1117
Copy link
Contributor

gui1117 commented Nov 15, 2024

It fails to compile:

   Using forklift, storage: gcs, compressor: zstd-3, uploader threads: 6
error[E0277]: the trait bound `ActiveRecovery<<<T as pallet::Config>::BlockNumberProvider as sp_runtime::traits::BlockNumberProvider>::BlockNumber, <<T as pallet::Config>::Currency as frame_support::traits::Currency<<T as frame_system::Config>::AccountId>>::Balance, _>: EncodeLike<ActiveRecovery<<<<T as frame_system::Config>::Block as sp_runtime::traits::Block>::Header as sp_runtime::traits::Header>::Number, <<T as pallet::Config>::Currency as frame_support::traits::Currency<<T as frame_system::Config>::AccountId>>::Balance, BoundedVec<<T as frame_system::Config>::AccountId, <T as pallet::Config>::MaxFriends>>>` is not satisfied
   --> substrate/frame/recovery/src/lib.rs:525:50
    |
525 |             <ActiveRecoveries<T>>::insert(&account, &who, recovery_status);
    |             -----------------------------                 ^^^^^^^^^^^^^^^ the trait `EncodeLike<ActiveRecovery<<<<T as frame_system::Config>::Block as sp_runtime::traits::Block>::Header as sp_runtime::traits::Header>::Number, <<T as pallet::Config>::Currency as frame_support::traits::Currency<<T as frame_system::Config>::AccountId>>::Balance, BoundedVec<<T as frame_system::Config>::AccountId, <T as pallet::Config>::MaxFriends>>>` is not implemented for `ActiveRecovery<<<T as Config>::BlockNumberProvider as BlockNumberProvider>::BlockNumber, ..., ...>`
    |             |
    |             required by a bound introduced by this call
    |
    = help: the trait `EncodeLike` is implemented for `ActiveRecovery<<<T as pallet::Config>::BlockNumberProvider as sp_runtime::traits::BlockNumberProvider>::BlockNumber, <<T as pallet::Config>::Currency as frame_support::traits::Currency<<T as frame_system::Config>::AccountId>>::Balance, _>`
note: required by a bound in `frame_support::pallet_prelude::StorageDoubleMap::<Prefix, Hasher1, Key1, Hasher2, Key2, Value, QueryKind, OnEmpty, MaxValues>::insert`
   --> /__w/polkadot-sdk/polkadot-sdk/substrate/frame/support/src/storage/types/double_map.rs:296:9
    |
292 |     pub fn insert<KArg1, KArg2, VArg>(k1: KArg1, k2: KArg2, val: VArg)
    |            ------ required by a bound in this associated function
...
296 |         VArg: EncodeLike<Value>,
    |               ^^^^^^^^^^^^^^^^^ required by this bound in `StorageDoubleMap::<Prefix, Hasher1, Key1, Hasher2, Key2, Value, QueryKind, OnEmpty, MaxValues>::insert`

error[E0308]: mismatched types
   --> substrate/frame/recovery/src/lib.rs:610:40
    |
610 |             ensure!(recoverable_block_number <= current_block_number, Error::<T>::DelayPeriod);
    |                                                 ^^^^^^^^^^^^^^^^^^^^ expected `sp_runtime::traits::Header::Number`, found `sp_runtime::traits::BlockNumberProvider::BlockNumber`
    |
    = note: expected associated type `<<<T as frame_system::Config>::Block as sp_runtime::traits::Block>::Header as sp_runtime::traits::Header>::Number`
               found associated type `<<T as pallet::Config>::BlockNumberProvider as sp_runtime::traits::BlockNumberProvider>::BlockNumber`
    = note: an associated type was expected, but a different one was found

@gui1117
Copy link
Contributor

gui1117 commented Nov 15, 2024

/cmd prdoc --audience runtime_dev --bump major

@gui1117
Copy link
Contributor

gui1117 commented Nov 15, 2024

ideally the prdoc should mention what is changing, and in which context a migration is needed.

You can take example on another PR:

      This PR adds the ability for the Scheduler pallet to specify its source of the block number.
      This is needed for the scheduler pallet to work on a parachain which does not produce blocks
      on a regular schedule, thus can use the relay chain as a block provider. Because blocks are
      not produced regularly, we cannot make the assumption that the block number increases 
      monotonically, and thus have a new logic via a `Queue` to handle multiple blocks with valid 
      agenda passed between them.

      This change only needs a migration for the `Queue`:
      1. If the `BlockNumberProvider` continues to use the system pallet's block number
      2. When a pallet deployed on the relay chain is moved to a parachain, but still uses the 
      relay chain's block number

      However, we would need migrations if the deployed pallets are upgraded on an existing parachain,
      and the `BlockNumberProvider` uses the relay chain block number.  

@paritytech-review-bot paritytech-review-bot bot requested a review from a team December 7, 2024 11:21
@bkchr bkchr added the T2-pallets This PR/Issue is related to a particular pallet. label Dec 12, 2024
@bkchr bkchr added this pull request to the merge queue Dec 22, 2024
Merged via the queue into paritytech:master with commit 88d900a Dec 22, 2024
198 of 199 checks passed
@aurexav aurexav deleted the blocknumberprovider-recovery branch December 23, 2024 01:38
dudo50 pushed a commit to paraspell-research/polkadot-sdk that referenced this pull request Jan 4, 2025
Make pallet-recovery supports `BlockNumberProvider`.

Part of paritytech#6297.

---

Polkadot address: 156HGo9setPcU2qhFMVWLkcmtCEGySLwNqa3DaEiYSWtte4Y

---------

Co-authored-by: Guillaume Thiolliere <[email protected]>
Co-authored-by: GitHub Action <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T2-pallets This PR/Issue is related to a particular pallet.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants