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

write a simulation for POA to get off SDK fork #170

Closed
Reecepbcups opened this issue Apr 24, 2024 · 29 comments
Closed

write a simulation for POA to get off SDK fork #170

Reecepbcups opened this issue Apr 24, 2024 · 29 comments

Comments

@Reecepbcups
Copy link
Member

Reecepbcups commented Apr 24, 2024 •
edited
Loading

write simulator for POA before he merges cosmos/cosmos-sdk#20059

another idea is to remove app.SlashingKeeper.Hooks(), from StakingKeeper hooks?

other idea is POA on new val, set signing info -1 block. then see if sdk patch required
@Reecepbcups
Copy link
Member Author

May not get us off a fork anyways since andrew needs PostSetupStandalone w/ app param in v50 anyways. Which cosmos/cosmos-sdk@7b35e36 only had part of.

Either way simulation is a good thing to add

@Reecepbcups Reecepbcups mentioned this issue Apr 26, 2024
Merged
@fmorency
Copy link
Contributor

fmorency commented May 8, 2024

Hey, I started writing simulation support in Manifest and encountered staking actions are now allowed on this chain.

I'm happy to help!

@Reecepbcups
Copy link
Member Author

Reecepbcups commented May 8, 2024 •
edited
Loading

This is a POA error in the ante. You can not directly use the StakingKeeper as flow is expected through POA.

May not be needed i have a POA refactor idea I am working on to remove some of the pitfalls with SDK stuff I ran into

@fmorency
Copy link
Contributor

fmorency commented May 9, 2024

Sounds good.

You might already know this, but this repo's (poa) simulation produces the same error.

@Reecepbcups
Copy link
Member Author

Reecepbcups commented May 9, 2024 •
edited
Loading

Yes. I did not re-wire any of the sims due to not using it. The new design will be many times easier to sim as well (hopefully)

@fmorency
Copy link
Contributor

fmorency commented May 9, 2024

Can I help with anything @Reecepbcups?

@fmorency
Copy link
Contributor

Hey @Reecepbcups, I just wanted to follow up on this issue quickly. Is there a workaround that I could use to write simulator tests on our POA chain?

@fmorency fmorency mentioned this issue Jun 12, 2024
Closed
@fmorency
Copy link
Contributor

fmorency commented Jun 12, 2024 •
edited
Loading

I've made some progress on this. I was able to run the simulator by disabling x/staking messages.

  1. Create a sim_params.json file with the following content 
    {
  "op_weight_msg_delegate": 0,
  "op_weight_msg_undelegate": 0,
  "op_weight_msg_begin_redelegate": 0,
  "op_weight_msg_cancel_unbonding_delegation": 0,
  "op_weight_msg_create_validator": 0,
  "op_weight_msg_edit_validator": 0
}
  2. Run the application simulator with 
    go test ./app -run TestFullAppSimulation \
  -Enabled=true \
  -NumBlocks=200 \
  -Genesis= \
  -Verbose=true \
  -Commit=true \
  -Period=5 \
  -Params=/path/to/sim_params.json \
  -v \
  -timeout 24h

This approach will set the weight of the x/staking module message to 0, effectively disabling that module simulation.

Note: The NewPOADisableStakingDecorator allows staking.MsgEditValidator messages but we should disable it for simulation as the commission rate never gets updated.

@fmorency fmorency mentioned this issue Jun 13, 2024
Merged
@fmorency
Copy link
Contributor

@Reecepbcups

I made some progress on the PR. MsgCreateValidator and MsgRemovePendingValidator are working just fine.

I'm currently implementing MsgSetPower and I am getting a panic after a while when executing the following message

msg := poatypes.MsgSetPower{
	Sender:           admin,
	ValidatorAddress: validator.OperatorAddress,
	Power:            1_000_000,
	Unsafe:           false,
}

The panic log is

:42PM ERR panic recovered in runTx err="recovered: calculated final stake for delegator cosmos1mf03f7gqya7u98fhmfh4kmj6qhsafefjdesges greater than current stake\n\tfinal stake:\t161933052469.000000000000000000\n\tcurrent stake:\t1000000.000000000000000000\nstack:\ngoroutine 13 [running]:\nruntime/debug.Stack()\n\t/home/fmorency/go1.22.2/src/runtime/debug/stack.go:24 +0x5e\ngithub.com/cosmos/cosmos-sdk/baseapp.NewBaseApp.newDefaultRecoveryMiddleware.func5({0x26cb360, 0xc0017fe420})\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:74 +0x25\ngithub.com/cosmos/cosmos-sdk/baseapp.NewBaseApp.newDefaultRecoveryMiddleware.newRecoveryMiddleware.func7({0x26cb360?, 0xc0017fe420?})\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:42 +0x2d\ngithub.com/cosmos/cosmos-sdk/baseapp.processRecovery({0x26cb360, 0xc0017fe420}, 0x0?)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:31 +0x2f\ngithub.com/cosmos/cosmos-sdk/baseapp.processRecovery({0x26cb360, 0xc0017fe420}, 0x0?)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:36 +0x53\ngithub.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runTx.func1()\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/baseapp.go:837 +0x156\npanic({0x26cb360?, 0xc0017fe420?})\n\t/home/fmorency/go1.22.2/src/runtime/panic.go:770 +0x132\ngithub.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.CalculateDelegationRewards({{0x3a80b80, 0xc000cbaf30}, {0x3acee90, 0xc001029aa0}, {0x3ab9660, 0xc000cc8f00}, {0x7fa490023ba0, 0xc000f5c508}, {0x3acd288, 0xc001027e00}, ...}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/keeper/delegation.go:177 +0x894\ngithub.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.withdrawDelegationRewards({{0x3a80b80, 0xc000cbaf30}, {0x3acee90, 0xc001029aa0}, {0x3ab9660, 0xc000cc8f00}, {0x7fa490023ba0, 0xc000f5c508}, {0x3acd288, 0xc001027e00}, ...}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/keeper/delegation.go:222 +0x26b\ngithub.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.WithdrawDelegationRewards({{0x3a80b80, 0xc000cbaf30}, {0x3acee90, 0xc001029aa0}, {0x3ab9660, 0xc000cc8f00}, {0x7fa490023ba0, 0xc000f5c508}, {0x3acd288, 0xc001027e00}, ...}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/keeper/keeper.go:127 +0x15e\ngithub.com/cosmos/cosmos-sdk/x/distribution/keeper.msgServer.WithdrawDelegatorReward({{{0x3a80b80, 0xc000cbaf30}, {0x3acee90, 0xc001029aa0}, {0x3ab9660, 0xc000cc8f00}, {0x7fa490023ba0, 0xc000f5c508}, {0x3acd288, 0xc001027e00}, ...}}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/keeper/msg_server.go:59 +0x269\ngithub.com/cosmos/cosmos-sdk/x/distribution/types._Msg_WithdrawDelegatorReward_Handler.func1({0x3ab57f8?, 0xc002b65020?}, {0x2af3540?, 0xc002b547c0?})\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/types/tx.pb.go:1173 +0xcb\ngithub.com/cosmos/cosmos-sdk/baseapp.(*MsgServiceRouter).registerMsgServiceHandler.func2.1({0x3ab56a8, 0xc002b6ae08}, {0xc001a6de00?, 0x411265?}, 0x358?, 0xc000cc52f0)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/msg_service_router.go:175 +0x8a\ngithub.com/cosmos/cosmos-sdk/x/distribution/types._Msg_WithdrawDelegatorReward_Handler({0x2c33720, 0xc0006c4a00}, {0x3ab56a8, 0xc002b6ae08}, 0x3429498, 0xc002b549c0)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/types/tx.pb.go:1175 +0x143\ngithub.com/cosmos/cosmos-sdk/baseapp.(*MsgServiceRouter).registerMsgServiceHandler.func2({{0x3ab5600, 0x553c6c0}, {0x3acf750, 0xc0018973c0}, {{0x0, 0x0}, {0x2c5b432, 0xe}, 0x7, {0x0, ...}, ...}, ...}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/msg_service_router.go:198 +0x37f\ngithub.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runMsgs(_, {{0x3ab5600, 0x553c6c0}, {0x3acf750, 0xc0018973c0}, {{0x0, 0x0}, {0x2c5b432, 0xe}, 0x7, ...}, ...}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/baseapp.go:1010 +0x1e6\ngithub.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runTx(0xc001016908, 0x7, {0xc00404bb80, 0x149, 0x149})\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/baseapp.go:948 +0x114c\ngithub.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).SimDeliver(0xc001016908, 0x2c5b432?, {0x3a926e8?, 0xc00156b680?})\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/test_helpers.go:38 +0xa5\ngithub.com/cosmos/cosmos-sdk/x/simulation.GenAndDeliverTx({0xc002796ab0, 0xc001016908, {0x3acd648, 0xc001977c40}, 0x0, {0x3a9e470, 0xc002b54640}, {0xc000cc4b70, 0x1, 0x1}, ...}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/simulation/util.go:117 +0x2f2\ngithub.com/cosmos/cosmos-sdk/x/simulation.GenAndDeliverTxWithRandFees({0xc002796ab0, 0xc001016908, {0x3acd648, 0xc001977c40}, 0x0, {0x3a9e470, 0xc002b54640}, {0xc000cc4b70, 0x1, 0x1}, ...})\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/simulation/util.go:96 +0x2d1\ngithub.com/cosmos/cosmos-sdk/x/distribution/simulation.WeightedOperations.SimulateMsgWithdrawDelegatorReward.func6(_, _, {{0x3ab5600, 0x553c6c0}, {0x3acf750, 0xc0019292c0}, {{0x0, 0x0}, {0x2c5b432, 0xe}, ...}, ...}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/simulation/operations.go:171 +0xad0\ngithub.com/cosmos/cosmos-sdk/x/simulation.createBlockSimulator.func1(_, _, {{0x3ab5600, 0x553c6c0}, {0x3acf750, 0xc0019292c0}, {{0x0, 0x0}, {0x2c5b432, 0xe}, ...}, ...}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/simulation/simulate.go:326 +0x512\ngithub.com/cosmos/cosmos-sdk/x/simulation.SimulateFromSeed({0x3af52b0, 0xc00103cb60}, {_, _}, _, _, _, {0xc001bf6008, 0x34, 0x3f}, ...)\n\t/home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/simulation/simulate.go:210 +0x19e4\ngithub.com/strangelove-ventures/poa/simapp.TestFullAppSimulation(0xc00103cb60)\n\t/home/fmorency/dev/poa/simapp/sim_test.go:82 +0x816\ntesting.tRunner(0xc00103cb60, 0x34289b8)\n\t/home/fmorency/go1.22.2/src/testing/testing.go:1689 +0xfb\ncreated by testing.(*T).Run in goroutine 1\n\t/home/fmorency/go1.22.2/src/testing/testing.go:1742 +0x390\n: panic"
Logs to writing to /home/fmorency/.simapp/simulations/1718394173108.log
    simulate.go:335: error on block  7/2000, operation (47/273) from x/distribution:
        recovered: calculated final stake for delegator cosmos1mf03f7gqya7u98fhmfh4kmj6qhsafefjdesges greater than current stake
                final stake:    161933052469.000000000000000000
                current stake:  1000000.000000000000000000
        stack:
        goroutine 13 [running]:
        runtime/debug.Stack()
                /home/fmorency/go1.22.2/src/runtime/debug/stack.go:24 +0x5e
        github.com/cosmos/cosmos-sdk/baseapp.NewBaseApp.newDefaultRecoveryMiddleware.func5({0x26cb360, 0xc0017fe420})
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:74 +0x25
        github.com/cosmos/cosmos-sdk/baseapp.NewBaseApp.newDefaultRecoveryMiddleware.newRecoveryMiddleware.func7({0x26cb360?, 0xc0017fe420?})
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:42 +0x2d
        github.com/cosmos/cosmos-sdk/baseapp.processRecovery({0x26cb360, 0xc0017fe420}, 0x0?)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:31 +0x2f
        github.com/cosmos/cosmos-sdk/baseapp.processRecovery({0x26cb360, 0xc0017fe420}, 0x0?)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:36 +0x53
        github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runTx.func1()
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/baseapp.go:837 +0x156
        panic({0x26cb360?, 0xc0017fe420?})
                /home/fmorency/go1.22.2/src/runtime/panic.go:770 +0x132
        github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.CalculateDelegationRewards({{0x3a80b80, 0xc000cbaf30}, {0x3acee90, 0xc001029aa0}, {0x3ab9660, 0xc000cc8f00}, {0x7fa490023ba0, 0xc000f5c508}, {0x3acd288, 0xc001027e00}, ...}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/keeper/delegation.go:177 +0x894
        github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.withdrawDelegationRewards({{0x3a80b80, 0xc000cbaf30}, {0x3acee90, 0xc001029aa0}, {0x3ab9660, 0xc000cc8f00}, {0x7fa490023ba0, 0xc000f5c508}, {0x3acd288, 0xc001027e00}, ...}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/keeper/delegation.go:222 +0x26b
        github.com/cosmos/cosmos-sdk/x/distribution/keeper.Keeper.WithdrawDelegationRewards({{0x3a80b80, 0xc000cbaf30}, {0x3acee90, 0xc001029aa0}, {0x3ab9660, 0xc000cc8f00}, {0x7fa490023ba0, 0xc000f5c508}, {0x3acd288, 0xc001027e00}, ...}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/keeper/keeper.go:127 +0x15e
        github.com/cosmos/cosmos-sdk/x/distribution/keeper.msgServer.WithdrawDelegatorReward({{{0x3a80b80, 0xc000cbaf30}, {0x3acee90, 0xc001029aa0}, {0x3ab9660, 0xc000cc8f00}, {0x7fa490023ba0, 0xc000f5c508}, {0x3acd288, 0xc001027e00}, ...}}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/keeper/msg_server.go:59 +0x269
        github.com/cosmos/cosmos-sdk/x/distribution/types._Msg_WithdrawDelegatorReward_Handler.func1({0x3ab57f8?, 0xc002b65020?}, {0x2af3540?, 0xc002b547c0?})
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/types/tx.pb.go:1173 +0xcb
        github.com/cosmos/cosmos-sdk/baseapp.(*MsgServiceRouter).registerMsgServiceHandler.func2.1({0x3ab56a8, 0xc002b6ae08}, {0xc001a6de00?, 0x411265?}, 0x358?, 0xc000cc52f0)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/msg_service_router.go:175 +0x8a
        github.com/cosmos/cosmos-sdk/x/distribution/types._Msg_WithdrawDelegatorReward_Handler({0x2c33720, 0xc0006c4a00}, {0x3ab56a8, 0xc002b6ae08}, 0x3429498, 0xc002b549c0)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/types/tx.pb.go:1175 +0x143
        github.com/cosmos/cosmos-sdk/baseapp.(*MsgServiceRouter).registerMsgServiceHandler.func2({{0x3ab5600, 0x553c6c0}, {0x3acf750, 0xc0018973c0}, {{0x0, 0x0}, {0x2c5b432, 0xe}, 0x7, {0x0, ...}, ...}, ...}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/msg_service_router.go:198 +0x37f
        github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runMsgs(_, {{0x3ab5600, 0x553c6c0}, {0x3acf750, 0xc0018973c0}, {{0x0, 0x0}, {0x2c5b432, 0xe}, 0x7, ...}, ...}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/baseapp.go:1010 +0x1e6
        github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).runTx(0xc001016908, 0x7, {0xc00404bb80, 0x149, 0x149})
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/baseapp.go:948 +0x114c
        github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).SimDeliver(0xc001016908, 0x2c5b432?, {0x3a926e8?, 0xc00156b680?})
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/baseapp/test_helpers.go:38 +0xa5
        github.com/cosmos/cosmos-sdk/x/simulation.GenAndDeliverTx({0xc002796ab0, 0xc001016908, {0x3acd648, 0xc001977c40}, 0x0, {0x3a9e470, 0xc002b54640}, {0xc000cc4b70, 0x1, 0x1}, ...}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/simulation/util.go:117 +0x2f2
        github.com/cosmos/cosmos-sdk/x/simulation.GenAndDeliverTxWithRandFees({0xc002796ab0, 0xc001016908, {0x3acd648, 0xc001977c40}, 0x0, {0x3a9e470, 0xc002b54640}, {0xc000cc4b70, 0x1, 0x1}, ...})
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/simulation/util.go:96 +0x2d1
        github.com/cosmos/cosmos-sdk/x/distribution/simulation.WeightedOperations.SimulateMsgWithdrawDelegatorReward.func6(_, _, {{0x3ab5600, 0x553c6c0}, {0x3acf750, 0xc0019292c0}, {{0x0, 0x0}, {0x2c5b432, 0xe}, ...}, ...}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/distribution/simulation/operations.go:171 +0xad0
        github.com/cosmos/cosmos-sdk/x/simulation.createBlockSimulator.func1(_, _, {{0x3ab5600, 0x553c6c0}, {0x3acf750, 0xc0019292c0}, {{0x0, 0x0}, {0x2c5b432, 0xe}, ...}, ...}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/simulation/simulate.go:326 +0x512
        github.com/cosmos/cosmos-sdk/x/simulation.SimulateFromSeed({0x3af52b0, 0xc00103cb60}, {_, _}, _, _, _, {0xc001bf6008, 0x34, 0x3f}, ...)
                /home/fmorency/go/pkg/mod/github.com/rollchains/cosmos-sdk@v0.50.6/x/simulation/simulate.go:210 +0x19e4
        github.com/strangelove-ventures/poa/simapp.TestFullAppSimulation(0xc00103cb60)
                /home/fmorency/dev/poa/simapp/sim_test.go:82 +0x816
        testing.tRunner(0xc00103cb60, 0x34289b8)
                /home/fmorency/go1.22.2/src/testing/testing.go:1689 +0xfb
        created by testing.(*T).Run in goroutine 1
                /home/fmorency/go1.22.2/src/testing/testing.go:1742 +0x390
        : panic [rollchains/cosmos-sdk@v0.50.6/baseapp/recovery.go:72]
        Comment: unable to deliver tx

You can reproduce by running

go test ./simapp -run TestFullAppSimulation \
    -Enabled=true \
    -NumBlocks=2000 \
    -Genesis= \
    -Verbose=true \
    -Commit=true \
    -Period=5 \
    -Params=/path/to/sim_params.json \
    -Seed=12 \
    -v \
    -timeout 24h

against my PR at commit 5558615.

Any idea what's going on? Do I need to craft a non-random genesis file to make it work?

@Reecepbcups
Copy link
Member Author

@fmorency just got back from travel so I do not have much time to look into this yet. Just throwing down quick ideas:

1)) rewards auto claim in cosmos (changing delegations auto claims rewards) so it may be changing the amounts since I have Distribution & Mint enabled in the app (doubt it but yea it can get you)

  1. Also note that stake updates take height + 2 blocks to persist. So anytime you set, you must wait 2 blocks. Failure to do so will incorrectly show the prev value

@fmorency
Copy link
Contributor

@Reecepbcups Thank you for your response.

I disabled MsgSetWithdrawAddress, MsgWithdrawDelegationReward and MsgWithdrawValidatorCommission from the x/distribution module as per your suggestion in 1) and the simulation (2000 blocks) didn't crash.

I don't fully understand the problem but I'm concerned that we could get such a crash in production.

WDYT?

@fmorency
Copy link
Contributor

Hey @Reecepbcups,

I'm currently debugging MsgSetPower in the simulator and I was wondering if you could provide some clarifications about block power.

The function k.GetStakingKeeper().GetLastTotalPower() and k.GetCachedBlockPower() return different values in SimulateMsgSetPower(). I also get a 3rd value if I compute the total power from all the validators.

My feeling is that I need to use k.GetCachedBlockPower() but I would like to better understand the block power flow.

Thanks!

@fmorency
Copy link
Contributor

fmorency commented Jun 19, 2024 •
edited
Loading

Hey @Reecepbcups,

PR #186 is ready for a first round of comments.

  1. I still get the crash mentioned in write a simulation for POA to get off SDK fork #170 (comment) if I don't have

      "op_weight_msg_withdraw_delegation_reward": 0

    present in my parameter file. I'm still concerned about this one for production.

  2. I made MsgSetPower work but would like to better understand the block/validator power flow.

Thanks!

@fmorency
Copy link
Contributor

fmorency commented Jul 2, 2024 •
edited
Loading

I've been debugging this thing. Here's an update.

  • CosmosSDK is not built to test multi-token scenarios; a lot of simulator-related stuff is hardcoded to use sdk.DefaultBondDenom
    • I have a local CosmosSDK branch allowing (some) simulator multi-token setup, e.g., mint_denom="notused", bond_denom="upoa", etc
    • Still need to change sdk.DefaultBondDenom in TestFullAppSimulation to upoa
  • The issue doesn't appear to come from slashing
  • Something somewhere is changing the number of shares returned by GetShares() used in CalculateDelegationRewards()

I'll continue debugging tomorrow.

Edit: Oh, it's the MsgSetPower!

@fmorency
Copy link
Contributor

fmorency commented Jul 4, 2024 •
edited
Loading

I think I fixed all the crashes. 4x simulator had been running in a loop for a while now.

I removed the x/distribution module from the app as it was causing problems. @Reecepbcups, can this removal cause undesired side effects, e.g., inability to migrate to PoS later?

Here's the params.json file I'm running the simulator with

{
  "op_weight_msg_delegate": 0,
  "op_weight_msg_undelegate": 0,
  "op_weight_msg_begin_redelegate": 0,
  "op_weight_msg_cancel_unbonding_delegation": 0,
  "op_weight_msg_create_validator": 0,
  "op_weight_msg_edit_validator": 0,

  "inflation": "0.000000000000000000",
  "annual_provisions": "0.000000000000000000",
  "inflation_rate_change": "0.000000000000000000",
  "inflation_max": "0.000000000000000000",
  "inflation_min": "0.000000000000000000",
  "goal_bonded": "0.670000000000000000",
  "blocks_per_year": "6311520",

  "slash_fraction_double_sign": "0.000000000000000000",
  "slash_fraction_downtime": "0.000000000000000000",

  "community_tax": "0.0"
}

@fmorency
Copy link
Contributor

fmorency commented Jul 4, 2024

I caught one crash caused by cosmos/cosmos-sdk#18196 which should be fixed in the next CosmosSDK version.

@fmorency
Copy link
Contributor

fmorency commented Jul 4, 2024

I caught a couple of group: not found [/home/fmorency/dev/cosmos-sdk/x/group/keeper/grpc_query.go:27] errors which I believe are fixed by cosmos/cosmos-sdk#17911. It should be fixed in the next CosmosSDK version.

@fmorency
Copy link
Contributor

fmorency commented Jul 5, 2024

I caught one crash caused by cosmos/cosmos-sdk#18196 which should be fixed in the next CosmosSDK version.

Backported to 0.50.x in cosmos/cosmos-sdk#20897

@Reecepbcups
Copy link
Member Author

@fmorency I'm not sure yet but I assume there is some unentended behavior yea. Since It’s a core sdk module, there is some logic that probably expects it at height 1. There may also be use cases where someone wants to distribute rewards to validators on POA network. Not sure if the SDK team will consider it fully sim-ed or not with a core distribution module being removed (which staking uses in their upstream sims no problem)

@fmorency
Copy link
Contributor

fmorency commented Jul 8, 2024

@Reecepbcups

Thanks for your feedback.

RE - There may also be use cases where someone wants to distribute rewards to validators on POA network.

I agree. However, the current state of the POA module is that I believe it's unsafe to use if the x/distribution module is enabled. I need your help and knowledge to make it work against the x/distribution module. Perhaps we could virtually meet and try to tackle this thing?

@fmorency
Copy link
Contributor

fmorency commented Jul 8, 2024 •
edited
Loading

The x/distribution contains the following check

https://github.com/cosmos/cosmos-sdk/blob/c4d9a495052b78a02723ed1df1d336efe83a4e8d/x/distribution/keeper/delegation.go#L112-L182

where the stakes are truncated when multiplied by slash fractions. The sanity check makes sure that the stakes (minus the slash fraction) agree with the tokens obtained from the delegation's shares.

When a POA validator power is updated, e.g., using MsgSetPower, the new validator values are set by

poa/keeper/poa.go

Lines 19 to 46 in da5cc79

func (k Keeper) UpdateValidatorSet(ctx context.Context, newShares, newConsensusPower int64, val stakingtypes.Validator, valAddr sdk.ValAddress) error {
newShare := sdkmath.LegacyNewDec(newShares)
newShareInt := sdkmath.NewIntFromUint64(uint64(newShares))
delAddr := sdk.AccAddress(valAddr.Bytes())
delegation := stakingtypes.Delegation{
DelegatorAddress: delAddr.String(),
ValidatorAddress: val.OperatorAddress,
Shares: newShare,
}
if err := k.stakingKeeper.SetDelegation(ctx, delegation); err != nil {
return err
}
val.Tokens = newShareInt
val.DelegatorShares = newShare
val.Status = stakingtypes.Bonded
if err := k.stakingKeeper.SetValidator(ctx, val); err != nil {
return err
}
if err := k.stakingKeeper.SetLastValidatorPower(ctx, valAddr, newConsensusPower); err != nil {
return err
}
return k.updateTotalPower(ctx)
}

When calculating delegation rewards

// x/distribution/keeper/delegation.go

startingInfo, err := k.GetDelegatorStartingInfo(ctx, sdk.ValAddress(valAddr), sdk.AccAddress(delAddr))
...
stake := startingInfo.Stake
...
currentStake := val.TokensFromShares(del.GetShares())
...

the stake variable contains the number of stakes from the beginning of the block, while the currentStake variable contains the updated value, e.g., from MsgSetPower.

The error will trigger if stake.GT(currentStake).

@fmorency
Copy link
Contributor

fmorency commented Jul 9, 2024

I caught a couple of group: not found [/home/fmorency/dev/cosmos-sdk/x/group/keeper/grpc_query.go:27] errors which I believe are fixed by cosmos/cosmos-sdk#17911. It should be fixed in the next CosmosSDK version.

Backported to 0.50.x in cosmos/cosmos-sdk#20909

@fmorency
Copy link
Contributor

@Reecepbcups

I ran simulations in a loop overnight using the PR with the x/distribution module removed. Everything was okay besides the two CosmosSDK issues mentioned above which were fixed and are about to be backported to 0.50.x.

I'm afraid I can't continue this work until I get some help from a POA/CosmosSDK dev.

@fmorency
Copy link
Contributor

Ref. cosmos/cosmos-sdk#20059 (comment)

@fmorency fmorency mentioned this issue Jul 10, 2024
Closed
12 tasks
@Reecepbcups
Copy link
Member Author

So if a val has tokens delegated, and they delegate more, they are auto claimed. So another option here is reduce minting to 0%, then distribution is effectively disabled without removing it (ideal) since this is for simulation anyways.

this way stake should remain the same and never auto claimed as values are updated and set etc

@fmorency
Copy link
Contributor

So if a val has tokens delegated, and they delegate more, they are auto claimed. So another option here is reduce minting to 0%, then distribution is effectively disabled without removing it (ideal) since this is for simulation anyways.

this way stake should remain the same and never auto claimed as values are updated and set etc

I believe minting is already set to 0% in the simulation parameter file, or are you talking about something else?

@Reecepbcups
Copy link
Member Author 

if you withdrqaw rewards, then it fails, distribution

Remove distribution from POA chains is fine, but upstreaming is the issue with simulation jail patch change
- [ ] Disable withdawing rewards with POA
- [ ] Distribution, on set power / remove, clear any disttribution tokens / feepool stuff so its not able to be withdrawn
- [ ] Feepool hack in the POA, where we clear that
- [ ] Staking calling Distribtution somewhere we are not missing

THIS IS LIKELY THE BEST SOLUTION
- [ ] H+2 issue?
    - [ ] If CacheBlockPower >0, PostHandler deny any withdraw messages
    - currently he does setpower + withdraw , set power + 2h then withdraw

DeleteDelegatorStartingInfo on SetPower

@fmorency
Copy link
Contributor

As discussed on Discord, we decided to implement an AnteHandler to prevent MsgWithdrawDelegatorRewards.

This fixes the issue while we work on a better POA design.

@Reecepbcups
Copy link
Member Author

Sim written & in tagged v0.50.2 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@fmorency @Reecepbcups