Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature fee address #1091

Merged
merged 6 commits into from
Jan 26, 2024
Merged

feature fee address #1091

merged 6 commits into from
Jan 26, 2024

Conversation

sampocs
Copy link
Collaborator

@sampocs sampocs commented Jan 26, 2024

Context

There's been some discussion on the best way to create the fee module account.

How we got here

  • The code on main was working on dockernet, but threw an error when initializing the chain for the testnet
  • It's not clear why it didn't work on both, but it appears that the genesis validation was being run independent from InitGenesis
  • When testing the .GetModuleAccount approach from informal - we were forgetting to register the module with maccPerms, which is why it was failing

Conclusion

  • The .GetModuleAccount approach does work if we add it to maccPerms
  • There's really no need to store this on the host zone if it's a normal module account - we can query it through the auth store
  • There's also no need to validate the address in genesis since it's a module account

Brief Changelog

  • Removed fee address from host zone
  • Added module account using .GetModuleAccount approach
  • Added module name to maccPerms
  • Updated LS and distribute logic to use new account
  • Fixed unit tests
  • Fixed dockernet scripts

Testing

  • Covered by unit test
  • Tested distribution to STRD stakers in dockernet

Copy link
Contributor

@asalzmann asalzmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, this seems cleaner!

x/staketia/keeper/genesis.go Show resolved Hide resolved
@sampocs sampocs added the A:automerge Automatically merge PR once checks pass label Jan 26, 2024
@mergify mergify bot merged commit 6a5dbc4 into main Jan 26, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once checks pass C:app-wiring C:proto
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants