Move `fill_block` to RootOffences

[Szegoo]

As mentioned in #11352 the fill_block function should be moved to the new root offences pallet, since it is only used for testing purposes.

This PR also contains some small nit fixes in the pallet that I originally haven't noticed.

[Szegoo]

@kianenigma Actually as I made this PR I am not sure if this function fits in this pallet. It doesn't have anything to do with the offences or staking pallet, I am interested in what you think about this.

Edit: One problem that occurs because of this is inside the utility pallet. It uses the fill_block function inside the tests. If we move the function to root-offences pallet then the mock runtime has to implement the staking pallet config, even though it has nothing to do with it.

[bkchr]

Yeah, I'm also not convinced that we should move this to RootOffences. We should think about if we need this at all? If we need this somewhere, we can just add this to some test pallet.

[Szegoo]

@bkchr Tests inside the utility pallet and node executor are the only places where fill_block is being used. Should we move this to the utility pallet, or did you have some other pallet in your mind?

[bkchr]

@bkchr Tests inside the utility pallet and node executor are the only places where fill_block is being used. Should we move this to the utility pallet, or did you have some other pallet in your mind?

What I meant is more, are there any "integration test" like tests outside of the code that use this. We probably will not find out before removing this :P

We could maybe add some new pallet that is suited for testing only. It would contain the fill_block extrinsic and maybe later other extrinsics as well.

[Szegoo]

Ok, so should we first remove this to see if it is used(can't this cause some problems if fill_block is being used?) or should I add a new testing pallet right away?

Edit: I believe that there would be some functions that could be moved to the testing pallet in the future.

[bkchr]

or should I add a new testing pallet right away?

I would vote for this.

[kianenigma]

I think it is worth exploring deleting the transaction as well. it is only needed to fill blocks in some integration test and there should be other ways to achieve the same.

[Szegoo]

there should be other ways to achieve the same.

@kianenigma Wouldn't this pollute the code in the tests though? Or do you have an idea which call would best fit this task?

[kianenigma]

Wouldn't this pollute the code in the tests though? Or do you have an idea which call would best fit this task?

For example if we change system::fill_block() by some other function in system pallet (that's actually test-only and not in the Call enum), I don't think it would pollute anything.

[Szegoo]

For example if we change system::fill_block() by some other function in system pallet (that's actually test-only and not in the Call enum), I don't think it would pollute anything.

Good idea, we could probably use set_block_consumed_resources for this.

Edit: I am not actually sure if set_block_consumed_resources could replace fill_block. set_block_consumed_resources sets the weight for the current existing block, while fill_block is being used in blocks that don't yet exist but are constructed in the test(At least in the substrate codebase, but I assume the integration tests if used, they use it in a similar way).

@kianenigma I don't think we can replace fill_block with a function that is not inside the Call enum.

cc: @bkchr

[Szegoo]

@kianenigma I created the new pallet, There are two tests in the polkadot repository(one in kusama runtime and the other one in polkadot) I assume we should remove these tests(the tests are for multiplier growth), or is there a workaround, because for sure we don't want to implement a testing pallet to kusama/polkadot.

cc: @bkchr

[bkchr]

@Szegoo CI is failing.

[Szegoo]

@bkchr When compiling kitchensink-runtime I get the following error: the wasm32-unknown-unknown target is not supported by default, you may need to enable the "js" feature.. I am not sure why am I getting this error, As far as I can tell I correctly defined the dependencies and configured std.

[kianenigma]

/tip medium

[substrate-tip-bot[bot]]

@kianenigma A medium tip was successfully submitted for Szegoo (126X27SbhrV19mBFawys3ovkyBS87SGfYwtwa8J2FjHrtbmA on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/treasury/tips

[bkchr]

@Szegoo CI still not happy :D

[Szegoo]

@bkchr The CI seems to be happy now :)

[bkchr]

Sorry @Szegoo, but Polkadot would like to have some companion :see_no_evil:

[Szegoo]

@kianenigma I created the new pallet, There are two tests in the polkadot repository(one in kusama runtime and the other one in polkadot) I assume we should remove these tests(the tests are for multiplier growth), or is there a workaround, because for sure we don't want to implement a testing pallet to kusama/polkadot.

cc: @bkchr

Yeah, but what should we do about this, should I remove these tests? @bkchr

[bkchr]

@kianenigma I created the new pallet, There are two tests in the polkadot repository(one in kusama runtime and the other one in polkadot) I assume we should remove these tests(the tests are for multiplier growth), or is there a workaround, because for sure we don't want to implement a testing pallet to kusama/polkadot. cc: @bkchr

Yeah, but what should we do about this, should I remove these tests? @bkchr

Yeah that is bad. Can you maybe change the test to fill up the block with balance transfers?

[Szegoo]

@bkchr The CI is happy now, this is probably ready to be merged then :)

[bkchr]

bot merge

[bkchr]

Ty for all the patience @Szegoo :)