1

orml-authority implements a mechanism to allow origin to dispatch calls after and only after a delay. This is implemented using a pallet origin DelayedOrigin.

The DealyedOrigin references the original origin so it can dispatch the call with the original origin later after the delay.

Something like:

enum RuntimeOrigin {
    System(frame_system::Origin),
    OtherPallet(other_pallet::Origin),
    Authority(orml_authoirty::DelayedOrigin<BlockNumber, RuntimeOrigin>),
}

struct DelayedOrigin<BlockNumber, RuntimeOrigin> {
    pub delay: BlockNumber,
    pub origin: Box<PalletsOrigin>,
}

With recent version of Substrate, it is required every storage item to have MaxEncodedLen.

The default implementation is something like:

impl MaxEncodedLen for RuntimeOrigin {
    fn max_encoded_len() -> usize {
        1 + max(
          frame_system::Origin::max_encoded_len(),
          other_pallet::Origin::max_encoded_len(),
          PalletsOrigin::<BlockNumber, RuntimeOrigin>::max_encoded_len()
        )
    }
}

impl <BlockNumber, RuntimeOrigin> MaxEncodedLen for DelayedOrigin<BlockNumber, RuntimeOrigin> {
    fn max_encoded_len() -> usize {
        BlockNumber::max_encoded_len() + RuntimeOrigin::max_encoded_len()
    }
}

As you can see, we have an infinite recursion here. Mostly because it is possible to construct an instance that is:

RuntimeOrigin::Authority(DelayedOrigin {
  delay: 0,
  origin: RuntimeOrigin::Authority(DelayedOrigin {
    delay: 0,
    origin: // keep nesting as much as we need
  })
})

Therefore the max encoded len is infinite or memory limit.

This wasn't an issue previously because the memory usage is O(n) to the tx size. As long as there is a reasonable tx byte fee, it is ok to nest as many as people wants.

However with recent Weight V2 change, we are required to know the upper size limit of a type. This requirements breaks the current working code.

It is not unreasonable to not support nested DelayedOrigin and therefore break the loop. However implementation wise I don't see an easy way.

The code will be something like:

impl MaxEncodedLen for RuntimeOrigin {
    fn max_encoded_len() -> usize {
        1 + max(
          frame_system::Origin::max_encoded_len(),
          other_pallet::Origin::max_encoded_len(),
          PalletsOrigin::<BlockNumber, RuntimeOrigin>::max_encoded_len()
        )
    }
}

impl RuntimeOrigin {
    fn max_encoded_len_except_authority() -> usize {
        1 + max(
          frame_system::Origin::max_encoded_len(),
          other_pallet::Origin::max_encoded_len(),
        )
    }
}

impl <BlockNumber, RuntimeOrigin> MaxEncodedLen for DelayedOrigin<BlockNumber, RuntimeOrigin> {
    fn max_encoded_len() -> usize {
        BlockNumber::max_encoded_len() + RuntimeOrigin::max_encoded_len_except_authority()
    }
}

impl<BlockNumber, PalletsOrigin> Decode for DelayedOrigin<BlockNumber, PalletsOrigin> {
    fn decode<I: codec::Input>(input: &mut I) -> Result<Self, codec::Error> {
        let delay = Decode::decode(input)?;
        let origin = Decode::decode(input)?;
        require_no_nesting(origin)?;
        Ok(DelayedOrigin { delay, origin })
    }
}

However, it is not currently possible to implement max_encoded_len_except_authority without modify the construct_runtime and it is simply no way to have the construct_runtime know what origin may contain nesting.

Also for require_no_nesting, there is also no easy way to implement it. One possible implementation is use global variable to assert no reentrancy. But it will be great if there is a more clean way.


Side question: What will go wrong if max_encoded_len is underestimating the number? i.e. when it returns 200 but the actual answer is max tx length.

2 Answers 2

3

You're right that this kind of construct is not directly supported by MaxEncodedLen because, just like Vec<u8>, it has no strict limit to the encoded length.

I wouldn't really trying to work around the MaxEncodedLen trait in this way as you're essentially undermining the protection given by the type system.

One possible direction is to adapt the existing macros to allow your runtime to declare a second RuntimeOrigin type called e.g. NonDelayedRuntimeOrigin. This would not contain the problematic DelayedOrigin variant. You can then have DelayedOrigin take an inner type of NonDelayedRuntimeOrigin, and you avoid the recursion completely.

What will go wrong if max_encoded_len is underestimating the number? i.e. when it returns 200 but the actual answer is max tx length.

Essentially the same thing as if the compute-time Weight component is under-estimated: Worst case it will lead to a block whose PoV size is necessarily greater than the maximum valid PoV for a block, which will brick the chain.

4
  • that will require some kind of plugin extension to the construct_runtime macro to allow us write some code to generate NonDelayedRuntimeOrigin?
    – Bryan Chen
    Nov 18 at 9:53
  • or perhaps just some sort of option to generate a second Origin type which skips some of the variants.
    – Gav
    Nov 18 at 10:39
  • Not very sure how will that to be implemented. Are you suggesting to add some optional parameters to the construct_runtime macro to achieve this?
    – Bryan Chen
    Nov 18 at 11:05
  • Yes; that would seem most efficient.
    – Gav
    Nov 19 at 10:20
0

I implemented the way using global variable to assert no reentrancy.

https://github.com/open-web3-stack/open-runtime-module-library/pull/845

impl<BlockNumber: Decode, PalletsOrigin: Decode> Decode for DelayedOrigin<BlockNumber, PalletsOrigin> {
    fn decode<I: codec::Input>(input: &mut I) -> Result<Self, codec::Error> {
        if helper::nested_decode() {
            return Err("Nested DelayedOrigin::decode is not allowed".into());
        }

        helper::set_nested_decode(true);
        defer!(helper::set_nested_decode(false));

        Ok(DelayedOrigin {
            delay: Decode::decode(input)?,
            origin: Decode::decode(input)?,
        })
    }
}

impl<BlockNumber: MaxEncodedLen, PalletsOrigin: MaxEncodedLen> MaxEncodedLen
    for DelayedOrigin<BlockNumber, PalletsOrigin>
{
    fn max_encoded_len() -> usize {
        if helper::nested_max_encoded_len() {
            return 0;
        }

        helper::set_nested_max_encoded_len(true);
        defer!(helper::set_nested_max_encoded_len(false));

        BlockNumber::max_encoded_len() + PalletsOrigin::max_encoded_len()
    }
}

This prevents user from submitting tx containing Orign with nested DealyedOrigin and make the max_encoded_len return 0 on the reentrancy call, which is fine because it shouldn't be possible to have nesting.

1
  • I tried to use const generics to attach a nested level to the Origin such that only a specific nesting limit is reachable, but Rust the features are not yet stable and it also did not work. A macro could be used to define limited nested types. Nov 21 at 16:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.