Add missing on_created_account logic usage for inc_account_nonce at pallet-evm-system

[dmitrylavrenov]

Discovered during work on #140

[MOZGIII]

This looks like a valid mode of operation - however, does it mirror what the frame system does? I assume so, as I don't see otherwise how you'd get the idea to do this.

This is a great work!

[dmitrylavrenov]

This looks like a valid mode of operation - however, does it mirror what the frame system does? I assume so, as I don't see otherwise how you'd get the idea to do this.

This is a great work!

Not exactly, it doesn't mirror what the frame system does (https://github.com/paritytech/substrate/blob/033d4e86cc7eff0066cd376b9375f815761d653c/frame/system/src/lib.rs#L1595).

The needs of it have been discovered during nonce increasing usage investigation at pallet-evm. Got that there are places that inc_account_nonce is called for accounts that haven't existed before. So, it's reasonable to call on_created_account exactly at inc_account_nonce as account is created after execution

[dmitrylavrenov]

The same is at polkadot-sdk master - https://github.com/paritytech/polkadot-sdk/blob/12ed0f4ffe4dcf3a8fe8928e3791141a110fad8b/substrate/frame/system/src/lib.rs#L2096.

on_created_account is called just at:

• inc_providers (https://github.com/paritytech/polkadot-sdk/blob/12ed0f4ffe4dcf3a8fe8928e3791141a110fad8b/substrate/frame/system/src/lib.rs#L1533)
• inc_sufficients (https://github.com/paritytech/polkadot-sdk/blob/12ed0f4ffe4dcf3a8fe8928e3791141a110fad8b/substrate/frame/system/src/lib.rs#L1591)

@MOZGIII Should we propose changes for inc_account_nonce logic to polkadot-sdk? As in theory, it's public available method that can be called at any time, even an account does't exist still. Nonce is increased after the call execution and the record is added?

Or it should be allowed to increase nonces as it doesn't related to general account existence (just a question). But on the other hand, the account itself is added to the state: so, we need to submit event about it at least...

[MOZGIII]

I'm thinking it should be covered, maybe, by the implementation of the Account::<T>::mutate itself? Such anything that calls into it, regardless of what the passed fn it, leads to the same unified behaviour with respect to emitting the event if the account was created?

This is really unfortunate the whole system is designed like a single flat list of calls, and there's no telling where acertain functionality would go at the type system layer. Would be nice if there were some layers added in there, with wrapper types... But this is something to propose as a broad design philosophy change to the upstream devs at parity.

[MOZGIII]

To reiterate: no doubt we need to fix this event, but the inc_account_nonce seems to be just an accessor/helper, while the system seems to allow other ways arbitrary to manipulate the nonce via Account::<T>::mutate - so that might have to be the place to address it.

[dmitrylavrenov]

Thanks for all your thoughts, share your point of view. But, I guess, it needs more time and research to propose some good philosophy change to the upstream devs at parity via possible Account::<T>::mutate.

I've double-checked all possible account mutation and creation at the current our simplified evm-system version => we've handled all cases.

As a result, we can merge this PR, I think.

[MOZGIII]

I'm thinking it should be covered, maybe, by the implementation of the Account::<T>::mutate itself? Such anything that calls into it, regardless of what the passed fn it, leads to the same unified behaviour with respect to emitting the event if the account was created?

This

This is really unfortunate the whole system is designed like a single flat list of calls, and there's no telling where acertain functionality would go at the type system layer. Would be nice if there were some layers added in there, with wrapper types... But this is something to propose as a broad design philosophy change to the upstream devs at parity.

Is not related to this.

Doing things via Account::<T>::mutate is not a big deal

[MOZGIII]

I've double-checked all possible account mutation and creation at the current our simplified evm-system version => we've handled all cases.

Not all possible cases though.