[Deprecation] Index vs Nonce

[AurevoirXavier]

Is there an existing issue?

• I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• This is not a support question.

Description of bug

Why do we need two different names for the same thing?

IMO this adds extra learning effort for developers.

use xxx::Index;
pub use xxx::Index as Nonce;

impl frame_system::Config {
  type Index = Index;
}

substrate_frame_rpc_system::AccountNonceApi<Block, AccountId, Nonce>;

---

I think maybe they are two different things.

Index is to describe the transaction index in the block.

Nonce is the account transactions counter.

And they are u32.

But the pub use as makes people confused.

---

Deprecation steps

Beta Give feedback

1. Soft deprecation and removal from the codebase
2. Update examples and tutorials Update examples and tutorials for deprecated Index polkadot-developers/substrate-docs#2035
3. Hard deprecation by adding a deprecate compiler warning
4. Deprecation is noted in the release log
5. Removal version is planned -> Next release
6. Deprecated feature is removed from the code base Replace system config Index for Nonce substrate#14290
7. Removal of the feature is noted in the release log

[Szegoo]

@AurevoirXavier I am probably wrong, but it seems like there aren't any instances of using an alias for Index/Nonce. And if I understand the issue correctly the problem you are mentioning is caused by using type aliases.

[AurevoirXavier]

@AurevoirXavier I am probably wrong, but it seems like there aren't any instances of using an alias for Index/Nonce. And if I understand the issue correctly the problem you are mentioning is caused by using type aliases.

In some repos, people alias the index to nonce.

As the example shows.

And in the Substrate repo:
We pass the Index to the Nonce field.
https://github.com/paritytech/substrate/blob/a1dee7ecd2a157c490d189f0932b32aa12106857/bin/node/rpc/src/lib.rs#L110

We also pass the Index to the Index field.
https://github.com/paritytech/substrate/blob/a1dee7ecd2a157c490d189f0932b32aa12106857/bin/node/runtime/src/lib.rs#L210

So, I start to think about are these two type the same thing.

Should we unify the name everywhere?

[burdges]

Aside: Nonce, aka number used only once, typically indicates a security break if you reuse the same number twice. I think nonce advancement cannot stop every possible attacks scenario, like if say two machines sign for a shared account on different forks of the chain, so nonce may warn of some niche scenarios. In principle, you could've stronger by coupling signing to consensus more tightly, but this would create more annoyances too.

[kianenigma]

@juangirini this is another simple deprecation task that we can pilot for a petter process.

Namely, it should be type Nonce in frame_system::Config, not type Index.

You can already see by the usage that this is a bit confusing:

https://github.com/paritytech/substrate/blob/bc3ce835964347166414203d68dccc26b9e9044f/frame/system/src/lib.rs#L1536-L1537

[juangirini]

Namely, it should be type Nonce in frame_system::Config, not type Index.

This would break backwards compatibility as existing Config implementations will be still using Index, and we can't set an alias or default value here. A work around would be to create a NewConfig (or whatever we want to call it) with the same types as Config except for the new Nonce instead of the Index type and deprecate Config. But not sure if we want to do this just for one renaming.

@kianenigma do you see an alternative solution?

[kianenigma]

Sadly this cannot be done backwards compatibly.

[juangirini]

I am not sure this renaming is worth the breaking change. If are sure we want to do it we rather do it sooner than later though.

@bkchr @ggwpez @kianenigma @KiChjang any thought on this?

[KiChjang]

I'm generally in favor of making sense of things. I think we've already made a bad precedent at times where people have to test every substrate upgrade -- on the list of annoyances, such a naming breaking change would list quite low IMHO, so yes, we should deprecate ASAP.

[xlc]

Rename is not bad compare to other breaking changes... It is just a search & replace and we all are used to this.

[KiChjang]

Fixed in paritytech/substrate#14290.

[juangirini]

This is still open awaiting for the last step of the deprecation todo list to be completed polkadot-developers/substrate-docs#2035