use counted maps instead of counters in authority members

pallet authority members has storage items like AuthoritiesCounter for counting elements. It is error prone and could be replaced by CountedStorageMap

changed milestone to %Horizon

changed milestone to %runtime-800

assigned to @c-geek

added D-easy P7-nicetohave labels

added RN-runtime label

created branch 148-use-counted-maps-instead-of-counters-in-authority-members to address this issue

mentioned in merge request !224 (merged)

We cannot use CountedStorageMap directly.

As far as I understand : AuthoritiesCounter = OnlineAuthorities.len() + IncomingAuthorities.len() - OutgoingAuthorities.len(). But these are vectors, not maps.

You are right. I think I added the issue too quickly after seeing that we have:

• 4 calls to AuthoritiesCounter::<T>::mutate
• 1 call to AuthoritiesCounter::<T>::get()

The only use of authorities counter is to check that go_online is allowed and we could instead compute it as you write it: OnlineAuthorities.len() + IncomingAuthorities.len() - OutgoingAuthorities.len().

Not a big deal though.

Well, you were right about the "error prone": there was a bug in the pallet (value should have been decremented instead of incremented): !224 (diffs)

It is even worse: the saturating_add had no effect on AuthoritiesCounter.

I've added a revealing test + fix.

mentioned in commit 58ba354f

mentioned in commit ec23a846

mentioned in commit 6c35d792

mentioned in commit 64bc018b

mentioned in commit 8ba0678d

mentioned in commit 0e9cf55d

mentioned in commit d62c033a

mentioned in commit 874d1c93

closed with merge request !224 (merged)