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.

Sign up for GitHub

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

Jump to bottom

feat!: simulator support #186

Merged
merged 24 commits into from
Jul 19, 2024
Merged

feat!: simulator support #186

merged 24 commits into from
Jul 19, 2024

Conversation

fmorency
Copy link
Contributor

@fmorency fmorency commented Jun 13, 2024 •
edited by Reecepbcups
Loading

Implement CosmosSDK simulator support for the POA module.

Breaking change: AnteHandler disabling MsgWithdrawDelegatorRewards from the x/distribution module.

Relates #170
Relates cosmos/cosmos-sdk#20664

Run with make sim-full-app (fixed seed) or make sim-full-app-random (random seed)

@fmorency fmorency marked this pull request as ready for review June 19, 2024 18:34
@fmorency fmorency mentioned this pull request Jun 19, 2024
Closed
@fmorency
Copy link
Contributor Author

fmorency commented Jun 19, 2024 •
edited
Loading

TestFullAppSimulation, TestAppSimulationAfterImport, and TestAppStateDeterminism all pass but TestAppImportExport fails. I'm not sure why.

Edit: TestAppImportExport fails when the application is using wiring v1. This behavior is also seen in CosmosSDK. A potential fix is to add support for wiring v2.

Ref #199

@fmorency fmorency mentioned this pull request Jun 19, 2024
Merged
@fmorency
Copy link
Contributor Author

@Reecepbcups I can't assign you as a reviewer. Commenting for visibility.

Reecepbcups
Reecepbcups reviewed Jun 26, 2024
View reviewed changes
keeper/keeper.go Outdated Show resolved Hide resolved
keeper/keeper_test.go Outdated Show resolved Hide resolved
@fmorency fmorency requested a review from Reecepbcups June 27, 2024 13:21
@fmorency
Copy link
Contributor Author

fmorency commented Jun 27, 2024 •
edited
Loading

The POA module has some incompatibility with the x/distribution core module.

A workaround is to remove the x/distribution module from your application.

See #170 (comment) for details.

Edit: Instead of removing the x/distribution module, we decided to deny MsgWithdrawDelegatorRewards message in the AnteHandler.

@fmorency fmorency mentioned this pull request Jul 16, 2024
Closed
@fmorency
Copy link
Contributor Author

Hey @Reecepbcups, I think we're good now.

@fmorency fmorency changed the title feat(simulator): simulator support feat!(simulator): simulator support Jul 19, 2024
@fmorency
Copy link
Contributor Author

Migrate to v0.50.8 when rollchains/cosmos-sdk#2 is merged and the rollchain version v0.50.8 is out. This version backports two patches fixing some simulator issues.

@Reecepbcups Reecepbcups changed the title feat!(simulator): simulator support feat!: simulator support Jul 19, 2024
Reecepbcups
Reecepbcups approved these changes Jul 19, 2024
View reviewed changes
Copy link
Member

@Reecepbcups Reecepbcups left a comment

Choose a reason for hiding this comment

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

Thank you so so much for your work here! SDK updated, tagging a release with info on the state changes

Reecepbcups
Reecepbcups reviewed Jul 19, 2024
View reviewed changes
ante/disable_withdraw_delegator_rewards.go

func (mdwr MsgDisableWithdrawDelegatorRewards) hasWithdrawDelegatorRewardsMsg(msgs []sdk.Msg) bool {
for _, msg := range msgs {
if _, ok := msg.(*distrtypes.MsgWithdrawDelegatorReward); ok {
Copy link
Member

Choose a reason for hiding this comment

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

technically there is a security issue here with authz nested bypasses. but since it is a trusted val set for POA, if someone really does that kinda obvious who did

@Reecepbcups Reecepbcups merged commit 93d500b into strangelove-ventures:main Jul 19, 2024
12 checks passed
@fmorency fmorency mentioned this pull request Jul 19, 2024
Closed
@fmorency fmorency deleted the simulation branch July 19, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Reecepbcups Reecepbcups Reecepbcups approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fmorency @Reecepbcups