[authority] Refactor AuthorityState creation to have a single creation point

[awelc]

This is ideally the first step towards running module initializers when creating genesis state.

The main point of this refactoring is to have a single AuthorityState constructor with a clearly defined set of modules that are created as part of the genesis state, so that we can run module initializers at the point of AuthorityState construction.

[lxfind]

I wonder if we still need the constructors that create AuthorityState without genesis.
We had those in the past because we didn't have genesis modules at first.
Now that we have them, I think they should always be included in all cases.

[awelc]

I wonder if we still need the constructors that create AuthorityState without genesis.

That's part of the refactoring. I got rid of the new_with_genesis_modules constructor so that we simply always use new - I left the new_without_genesis_for_testing as it's testing only so I did not bother.

[awelc]

I wonder if we still need the constructors that create AuthorityState without genesis.

That's part of the refactoring. I got rid of the new_with_genesis_modules constructor so that we simply always use new - I left the new_without_genesis_for_testing as it's testing only so I did not bother.

I removed the test-only constructor. Are we good with this refactoring otherwise?

[lxfind]

Do we still need two constructors? I was wondering if we could just always init it with genesis, and then we could just handle the module init logic in authority state constructor.

[awelc]

Do we still need two constructors? I was wondering if we could just always init it with genesis, and then we could just handle the module init logic in authority state constructor.

This is the intent I expressed in this PR's description, and my previous commit removed the test-only constructor (that would not have any genesis modules anyway), but even previously we only had one constructor for non-test cases. Am I missing something here?

[lxfind]

Ah sorry I misunderstood somehow!
In that case, why do we prefer passing the genesis module as parameter instead of getting it inside the constructor?

[awelc]

Ah sorry I misunderstood somehow! In that case, why do we prefer passing the genesis module as parameter instead of getting it inside the constructor?

This is because the genesis state is constructed slightly differently in genesir.rs and in sui_commands.rs and I did not intend it as too general of a refactoring, but rather just good enough to get me a single constructor for injecting module initializer execution.

[awelc]

Could I please get additional revision feedback or get it it stamped?

[lxfind]

Approving to unblock you.
But I don't quite understand why we have a path to create the state without genesis modules in sui_command. Maybe @patrickkuo or @oxade could clarify that.

[awelc]

Approving to unblock you. But I don't quite understand why we have a path to create the state without genesis modules in sui_command. Maybe @patrickkuo or @oxade could clarify that

Thank you! And I don't think we do a no-genesis part in sui_command. The separate genesis_modules parameter is there more because sui_command.rs creates a list of modules differently than adapter's genesis.rs (and none of this code was written by me) - the former creates them on-the-fly including user-defined modules whereas the latter stores them in a global so that it can be easily retrieved. And again, this refactoring was not meant to clean all this up but rather do enough to serve as the first step for module initialization code injection.

[lxfind]

Oops, forgot to delete this?

[awelc]

I certainly did... Thanks for noticing! Fixed in #518.