Set PoV in frontier template

[rimbi]

I have copied the settings from the node-template in frontier repo.

[tmpolaczyk]

I always forget how does saturating_div differ from normal div, so in case anyone is curious:

For unsigned types, it's exactly the same as normal div. It still panics on division by 0.

For signed types, it's the same except one edge case: i8::MIN / -1 returns i8::MAX instead of overflowing (because -256 is a valid value but +256 is not, so it returns +255 instead).

In this case it's a u64 so it's the same as using /, but you can keep it for symmetry with the other operations.

[tmpolaczyk]

Any chance we can test this?

[rimbi]

Any chance we can test this?

I am looking for an example test now. Are you aware of any?

[tmpolaczyk]

Maybe one of these? https://github.com/moonbeam-foundation/moonbeam/tree/master/test/suites/dev/test-pov

[tmpolaczyk]

FYI we merged some CI changes and this has conflicts, @rimbi you should merge master and run pnpm fmt:fix and pnpm lint:fix

[rimbi]

@tmpolaczyk @girazoki The tests pass now. But I am still not sure if they are enough to cover the change. What is your take on it?

The tests:
test/suites/dev-frontier-template/test-pov/

[girazoki]

Removing the expects should not be a solution for this. We need to investigate why the rpc is not returning the proof size

[rimbi]

Is it expected to return proof_size? Could be that it is missing the functionality. @librelois had a PR for proof_size on the moonbeam. Is it because tanssi lack that?

[librelois]

Could be that it is missing the functionality. @librelois had a PR for proof_size on the moonbeam. Is it because tanssi lack that?

yes, this feature was not merged upstream, you need to cherry-pick this commit on your substrate fork: moonbeam-foundation/substrate@0506153

[girazoki]

Thanks @librelois, @rimbi I will cherry-pick this change and see if it works

[girazoki]

We added the cherry-pick @rimbi, you need to change the commit in the cargo lock from 9202622403b4ecb4949ba48a3f8ba117d2f3b9bd to 1e3d44ae84b24ebfcf0850bbc49c962400dc0669

[rimbi]

@girazoki the response still lacks proofSize. I am building the node with build --release -p tanssi-node.

[tmpolaczyk]

It works for me locally, make sure to compile with cargo build --release --features fast-runtime

[rimbi]

I have manipulated the numbers in the tests so that the tests pass. I don't know if they make sense or not. Please check my last commit. @girazoki @tmpolaczyk

[rimbi]

Can we merge this?

[girazoki]

Nope, ideally we would want to know why the values we had originally (same as Moonbeam) dont work. It might well be that the pov is not being accounted in the EVM and that is why you had to lower the values

[girazoki]

I am requesting changes so that we dont forget to look at this

[girazoki]

This line is specially suspicious, 1M gas should indeed account for 250K pov proof, and our tests clearly indicate they dont

[girazoki]

So I investigated a bit more and I realizde your txs are failing (you can do pnpm moonwall run dev_frontier_tempalte and then grepTest the test that you want to run). I see all of them fail with OutOfGas, but if we remove the pov stuff, they dont fail. So the low numbers are probably because it fails

[girazoki]

More on this, is the estimateGas that it's failing. If the gas was well estimated, things would work. But seems estimateGas is not estimating the pov