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

simulation uses secp256k1 as validator keys #7009

Closed
4 tasks
tac0turtle opened this issue Aug 11, 2020 · 5 comments
Closed
4 tasks

simulation uses secp256k1 as validator keys #7009

tac0turtle opened this issue Aug 11, 2020 · 5 comments
Assignees

Comments

@tac0turtle
Copy link
Member

The simulator uses secp256k1 keys throughout. This was fine before because tendermint registered the secp256k1 key with amino. This is no longer the case making sims fail because the create and update valdiator functions use tendermint's PubkeyToProto function which does not contain secp256k1 in the switch statement

Steps to Reproduce

Run make test-sim-nondeterminism on branch tm_update_final it will panic within one second


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@aaronc aaronc added this to the v0.40 [Stargate] milestone Aug 11, 2020
@alexanderbez
Copy link
Contributor

alexanderbez commented Aug 11, 2020

For reference, my initial hunch was to modify x/staking/simulation/operations.go to create a new edwards key consensus key pair instead of using the account:

--- a/x/staking/simulation/genesis.go
+++ b/x/staking/simulation/genesis.go
@@ -7,6 +7,8 @@ import (
        "math/rand"
        "time"
 
+       "github.com/tendermint/tendermint/crypto/ed25519"
+
        "github.com/cosmos/cosmos-sdk/codec"
 
        sdk "github.com/cosmos/cosmos-sdk/types"
@@ -85,7 +87,11 @@ func RandomizedGenState(simState *module.SimulationState) {
                        simulation.RandomDecAmount(simState.Rand, maxCommission),
                )
 
-               validator := types.NewValidator(valAddr, simState.Accounts[i].PubKey, types.Description{})
+               privkeySeed := make([]byte, 15)
+               simState.Rand.Read(privkeySeed)
+               privKey := ed25519.GenPrivKeyFromSecret(privkeySeed)
+
+               validator := types.NewValidator(valAddr, privKey.PubKey(), types.Description{})
                validator.Tokens = sdk.NewInt(simState.InitialStake)
                validator.DelegatorShares = sdk.NewDec(simState.InitialStake)
                validator.Commission = commission

I've tested this change on master and simulations pass (I only tested one seed).

However, Marko reported that this still leads to issues when newMockValidators is called during initChain???

@tac0turtle
Copy link
Member Author

with the above change the error changes to

Simulating... block 3/100, operation 600/630.     mock_tendermint.go:95: tried to delete a nonexistent validator

@alexanderbez
Copy link
Contributor

mhhh that is odd, ok so instead I suggest accounts possibly have a consensus key?

diff --git a/types/simulation/account.go b/types/simulation/account.go
index 21bffd05f..372cb5831 100644
--- a/types/simulation/account.go
+++ b/types/simulation/account.go
@@ -4,6 +4,7 @@ import (
        "math/rand"
 
        "github.com/tendermint/tendermint/crypto"
+       "github.com/tendermint/tendermint/crypto/ed25519"
        "github.com/tendermint/tendermint/crypto/secp256k1"
 
        sdk "github.com/cosmos/cosmos-sdk/types"
@@ -13,9 +14,9 @@ import (
 // eventually more useful data can be placed in here.
 // (e.g. number of coins)
 type Account struct {
-       PrivKey crypto.PrivKey
-       PubKey  crypto.PubKey
-       Address sdk.AccAddress
+       PrivKey          crypto.PrivKey
+       ConsensusPrivKey crypto.PrivKey
+       Address          sdk.AccAddress
 }
 
 // Equals returns true if two accounts are equal
@@ -38,10 +39,15 @@ func RandomAccounts(r *rand.Rand, n int) []Account {
                // don't need that much entropy for simulation
                privkeySeed := make([]byte, 15)
                r.Read(privkeySeed)
+               privKey := secp256k1.GenPrivKeySecp256k1(privkeySeed)
 
-               accs[i].PrivKey = secp256k1.GenPrivKeySecp256k1(privkeySeed)
-               accs[i].PubKey = accs[i].PrivKey.PubKey()
-               accs[i].Address = sdk.AccAddress(accs[i].PubKey.Address())
+               privkeySeed = make([]byte, 15)
+               r.Read(privkeySeed)
+               consPrivKey := ed25519.GenPrivKeyFromSecret(privkeySeed)
+
+               accs[i].PrivKey = privKey
+               accs[i].ConsensusPrivKey = consPrivKey
+               accs[i].Address = sdk.AccAddress(privKey.PubKey().Address())
        }
 
        return accs
diff --git a/x/staking/simulation/genesis.go b/x/staking/simulation/genesis.go
index ffe07f680..478f171b0 100644
--- a/x/staking/simulation/genesis.go
+++ b/x/staking/simulation/genesis.go
@@ -85,7 +85,7 @@ func RandomizedGenState(simState *module.SimulationState) {
                        simulation.RandomDecAmount(simState.Rand, maxCommission),
                )
 
-               validator := types.NewValidator(valAddr, simState.Accounts[i].PubKey, types.Description{})
+               validator := types.NewValidator(valAddr, simState.Accounts[i].ConsensusPrivKey.PubKey(), types.Description{})
                validator.Tokens = sdk.NewInt(simState.InitialStake)
                validator.DelegatorShares = sdk.NewDec(simState.InitialStake)
                validator.Commission = commission
diff --git a/x/staking/simulation/operations.go b/x/staking/simulation/operations.go
index 116abe044..0c7c4a3d1 100644
--- a/x/staking/simulation/operations.go
+++ b/x/staking/simulation/operations.go
@@ -148,8 +148,7 @@ func SimulateMsgCreateValidator(ak types.AccountKeeper, bk types.BankKeeper, k k
                        simtypes.RandomDecAmount(r, maxCommission),
                )
 
-               msg := types.NewMsgCreateValidator(address, simAccount.PubKey,
-                       selfDelegation, description, commission, sdk.OneInt())
+               msg := types.NewMsgCreateValidator(address, simAccount.ConsensusPrivKey.PubKey(), selfDelegation, description, commission, sdk.OneInt())
 
                txGen := simappparams.MakeEncodingConfig().TxConfig
                tx, err := helpers.GenTx(

@clevinson
Copy link
Contributor

@marbar3778 i'll take a look at this tomorrow and see what i cna figure out.

@alexanderbez alexanderbez self-assigned this Aug 13, 2020
@alexanderbez
Copy link
Contributor

Fixed via a84b13aed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants