chore: simplify ADR-028 and address.Module

[robert-zaremba]

Description

Closes: #13910

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.

I have...

• [ ] included the correct type prefix in the PR title
• [ ] added ! to the type prefix if API or client breaking change
• [ ] targeted the correct branch (see PR Targeting)
• [ ] provided a link to the relevant issue or specification
• [ ] followed the guidelines for building modules
• [ ] included the necessary unit and integration tests
• [ ] added a changelog entry to CHANGELOG.md
• [ ] included comments for documenting Go code
• [ ] updated the relevant documentation or specification
• [ ] reviewed "Files changed" and left comments if necessary
• [ ] confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.

I have...

• [ ] confirmed the correct type prefix in the PR title
• [ ] confirmed ! in the type prefix if API or client breaking change
• [ ] confirmed all author checklist items have been addressed
• [ ] reviewed state machine logic
• [ ] reviewed API design and naming
• [ ] reviewed documentation is accurate
• [ ] reviewed tests and test coverage
• [ ] manually tested (if applicable)

[robert-zaremba]

Adding 0.47 backport.

[aaronc]

@amaurym would be great to get your review of this too.

[robert-zaremba]

Great, so waiting for @aaronc re-review. I will message him.

[alexanderbez]

@aaronc let us know when you can take a look at this again -- looks like we're blocked on your review :)

[robert-zaremba]

I've implemented the special case discussed in: https://github.com/cosmos/cosmos-sdk/pull/14017#discussion_r1052639955 commit: 67d8d8d940

[robert-zaremba]

up @aaronc , @amaurym

[robert-zaremba]

I'm fine with address.Module reverting to the pre-ADR 028 20-byte addresses (compatible with existing module accounts) when there are no derivation keys.

Personally, I would prefer to have only the "full" ADR-28 addresses for root module accounts. But simplification and compatibility is more important here.

[aaronc]

I'm fine with address.Module reverting to the pre-ADR 028 20-byte addresses (compatible with existing module accounts) when there are no derivation keys.

Personally, I would prefer to have only the "full" ADR-28 addresses for root module accounts. But simplification and compatibility is more important here.

For security purposes? All root accounts should be initialized on startup

[robert-zaremba]

For security purposes?

Mostly for having one general solution (with clear domains - what ADR-028 is mostly about).

I think security is OK here, because we have that module name hash. The concern is if someone is doing something weird and creating a module name which is a hash and could collide with an existing public key hash. Or generates multiple module accounts rather than deriving.

[sonarqubecloud[bot]]

[Cosmos SDK] Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
No Duplication information

[julienrbrt]

I believe it closes https://github.com/cosmos/cosmos-sdk/issues/13782 too right?

[julienrbrt]

Can we add a changelog? Then, given the discussion from the community call, we can add automerge.