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

fix: genesis account creation in e2e tests #3243

Merged
merged 2 commits into from
Apr 3, 2024
Merged

fix: genesis account creation in e2e tests #3243

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc929b9
fix genesis account creation in e2e tests
cmwaters Apr 2, 2024
d29846c
Merge branch 'main' into cal/genesis-account-e2e
cmwaters Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/test/priority_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (s *PriorityTestSuite) SetupSuite() {
require.NoError(t, cctx.WaitForNextBlock())

for _, acc := range cfg.Genesis.Accounts() {
addr := testfactory.GetAddress(s.cctx.Keyring, acc.Name)
addr := sdk.AccAddress(acc.PubKey.Address())
signer, err := user.SetupSigner(s.cctx.GoContext(), s.cctx.Keyring, s.cctx.GRPCClient, addr, s.ecfg)
signer.SetPollTime(time.Millisecond * 300)
require.NoError(t, err)
Expand Down
18 changes: 2 additions & 16 deletions test/e2e/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ import (

"github.com/celestiaorg/celestia-app/test/util/genesis"
"github.com/celestiaorg/knuu/pkg/knuu"
"github.com/cosmos/cosmos-sdk/crypto/hd"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
serverconfig "github.com/cosmos/cosmos-sdk/server/config"
"github.com/tendermint/tendermint/config"
"github.com/tendermint/tendermint/crypto"
Expand Down Expand Up @@ -37,7 +34,6 @@ type Node struct {
InitialPeers []string
SignerKey crypto.PrivKey
NetworkKey crypto.PrivKey
AccountPubKey cryptotypes.PubKey
SelfDelegation int64
Instance *knuu.Instance

Expand All @@ -51,7 +47,6 @@ func NewNode(
peers []string,
signerKey, networkKey crypto.PrivKey,
upgradeHeight int64,
keys keyring.Keyring,
grafana *GrafanaInfo,
) (*Node, error) {
instance, err := knuu.NewInstance(name)
Expand All @@ -62,14 +57,6 @@ func NewNode(
if err != nil {
return nil, err
}
record, _, err := keys.NewMnemonic(name, keyring.English, "", "", hd.Secp256k1)
if err != nil {
return nil, err
}
pubKey, err := record.GetPubKey()
if err != nil {
return nil, err
}

if err := instance.AddPortTCP(rpcPort); err != nil {
return nil, err
Expand Down Expand Up @@ -125,7 +112,6 @@ func NewNode(
InitialPeers: peers,
SignerKey: signerKey,
NetworkKey: networkKey,
AccountPubKey: pubKey,
SelfDelegation: selfDelegation,
}, nil
}
Expand Down Expand Up @@ -260,8 +246,8 @@ func (n *Node) Start() error {

func (n *Node) GenesisValidator() genesis.Validator {
return genesis.Validator{
Account: genesis.Account{
PubKey: n.AccountPubKey,
KeyringAccount: genesis.KeyringAccount{
staheri14 marked this conversation as resolved.
Show resolved Hide resolved
Name: n.Name,
InitialTokens: n.SelfDelegation,
},
ConsensusKey: n.SignerKey,
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/testnet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (t *Testnet) SetConsensusParams(params *tmproto.ConsensusParams) {
func (t *Testnet) CreateGenesisNode(version string, selfDelegation, upgradeHeight int64) error {
signerKey := t.keygen.Generate(ed25519Type)
networkKey := t.keygen.Generate(ed25519Type)
node, err := NewNode(fmt.Sprintf("val%d", len(t.nodes)), version, 0, selfDelegation, nil, signerKey, networkKey, upgradeHeight, t.genesis.Keyring(), t.grafana)
node, err := NewNode(fmt.Sprintf("val%d", len(t.nodes)), version, 0, selfDelegation, nil, signerKey, networkKey, upgradeHeight, t.grafana)
if err != nil {
return err
}
Expand All @@ -68,7 +68,7 @@ func (t *Testnet) CreateGenesisNodes(num int, version string, selfDelegation, up
func (t *Testnet) CreateNode(version string, startHeight, upgradeHeight int64) error {
signerKey := t.keygen.Generate(ed25519Type)
networkKey := t.keygen.Generate(ed25519Type)
node, err := NewNode(fmt.Sprintf("val%d", len(t.nodes)), version, startHeight, 0, nil, signerKey, networkKey, upgradeHeight, t.genesis.Keyring(), t.grafana)
node, err := NewNode(fmt.Sprintf("val%d", len(t.nodes)), version, startHeight, 0, nil, signerKey, networkKey, upgradeHeight, t.grafana)
if err != nil {
return err
}
Expand All @@ -88,8 +88,8 @@ func (t *Testnet) CreateAccount(name string, tokens int64) (keyring.Keyring, err
return nil, err
}
err = t.genesis.AddAccount(genesis.Account{
PubKey: pk,
InitialTokens: tokens,
PubKey: pk,
Balance: tokens,
})
if err != nil {
return nil, err
Expand Down
35 changes: 11 additions & 24 deletions test/util/genesis/accounts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/cosmos/cosmos-sdk/client/tx"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
"github.com/tendermint/tendermint/crypto"
Expand All @@ -20,40 +19,28 @@ const (
DefaultInitialBalance = 1e15 // 1 billion TIA
)

// Account represents a user account on the Celestia network.
// KeyringAccount represents a user account on the Celestia network.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps this part of the comment does not apply anymore:

or an address needs to be provided

Copy link
Collaborator

Choose a reason for hiding this comment

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

[suggestion] The name account in KeyringAccount is a bit confusing, as it is more of some info about the account not the actual account. I'd suggest using an alternative name like KeyringAccountInfo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I tried to distinguish KeyringAccount as an Account that is paired with a Keyring belonging to the Genesis struct. If you don't want to use the keyring provided by the Genesis struct then you use an Account and therefore must include the public key

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, your explanation has made things clearer. I think it would be helpful to also add these explanations as a comment to both structs.

// Either the name, if using the genesis keyring, or an address
// needs to be provided
type Account struct {
type KeyringAccount struct {
Comment on lines +22 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider revising the comment for KeyringAccount to reflect the removal of the address requirement.

// KeyringAccount represents a user account on the Celestia network.
// Either the name, if using the genesis keyring
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// KeyringAccount represents a user account on the Celestia network.
// Either the name, if using the genesis keyring, or an address
// needs to be provided
type Account struct {
type KeyringAccount struct {
// KeyringAccount represents a user account on the Celestia network.
// Either the name, if using the genesis keyring
type KeyringAccount struct {

Name string
PubKey cryptotypes.PubKey
InitialTokens int64
}

func NewAccounts(initBal int64, names ...string) []Account {
accounts := make([]Account, len(names))
func NewAccounts(initBal int64, names ...string) []KeyringAccount {
accounts := make([]KeyringAccount, len(names))
for i, name := range names {
accounts[i] = Account{
accounts[i] = KeyringAccount{
Name: name,
InitialTokens: initBal,
}
}
return accounts
}

func NewAccountsFromPubKeys(initBal int64, pubKeys ...cryptotypes.PubKey) []Account {
accounts := make([]Account, len(pubKeys))
for i, pubKey := range pubKeys {
accounts[i] = Account{
PubKey: pubKey,
InitialTokens: initBal,
}
}
return accounts
}

func (ga *Account) ValidateBasic() error {
if ga.Name == "" && ga.PubKey == nil {
return fmt.Errorf("both name and pubkey cannot be empty")
func (ga *KeyringAccount) ValidateBasic() error {
if ga.Name == "" {
return fmt.Errorf("name cannot be empty")
}
if ga.InitialTokens <= 0 {
return fmt.Errorf("initial tokens must be positive")
Expand All @@ -62,7 +49,7 @@ func (ga *Account) ValidateBasic() error {
}

type Validator struct {
Account
KeyringAccount
Stake int64

// ConsensusKey is the key used by the validator to sign votes.
Expand All @@ -73,7 +60,7 @@ type Validator struct {
func NewDefaultValidator(name string) Validator {
r := mrand.New(mrand.NewSource(time.Now().UnixNano()))
return Validator{
Account: Account{
KeyringAccount: KeyringAccount{
Name: name,
InitialTokens: DefaultInitialBalance,
},
Expand All @@ -85,7 +72,7 @@ func NewDefaultValidator(name string) Validator {

// ValidateBasic performs stateless validation on the validitor
func (v *Validator) ValidateBasic() error {
if err := v.Account.ValidateBasic(); err != nil {
if err := v.KeyringAccount.ValidateBasic(); err != nil {
return err
}
if v.Stake <= 0 {
Expand Down
18 changes: 17 additions & 1 deletion test/util/genesis/document.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/celestiaorg/celestia-app/app"
"github.com/celestiaorg/celestia-app/app/encoding"
"github.com/celestiaorg/celestia-app/pkg/appconsts"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
Expand Down Expand Up @@ -96,7 +97,7 @@ func accountsToSDKTypes(accounts []Account) ([]banktypes.Balance, []authtypes.Ge
hasMap[addr.String()] = struct{}{}

balances := sdk.NewCoins(
sdk.NewCoin(appconsts.BondDenom, sdk.NewInt(DefaultInitialBalance)),
sdk.NewCoin(appconsts.BondDenom, sdk.NewInt(account.Balance)),
)

genBals[i] = banktypes.Balance{Address: addr.String(), Coins: balances.Sort()}
Expand All @@ -105,3 +106,18 @@ func accountsToSDKTypes(accounts []Account) ([]banktypes.Balance, []authtypes.Ge
}
return genBals, genAccs, nil
}

type Account struct {
PubKey cryptotypes.PubKey
Balance int64
}

func (ga Account) ValidateBasic() error {
if ga.PubKey == nil {
return fmt.Errorf("pubkey cannot be empty")
}
if ga.Balance <= 0 {
return fmt.Errorf("balance must be greater than 0")
}
return nil
}
57 changes: 37 additions & 20 deletions test/util/genesis/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package genesis

import (
"bytes"
"encoding/json"
"fmt"
"time"
Expand Down Expand Up @@ -87,36 +88,52 @@ func (g *Genesis) WithValidators(vals ...Validator) *Genesis {
return g
}

func (g *Genesis) WithAccounts(accs ...Account) *Genesis {
func (g *Genesis) WithAccounts(accs ...KeyringAccount) *Genesis {
for _, acc := range accs {
err := g.AddAccount(acc)
err := g.NewAccount(acc)
if err != nil {
panic(err)
}
}
return g
}

func (g *Genesis) AddAccount(acc Account) error {
func (g *Genesis) AddAccount(account Account) error {
for _, acc := range g.accounts {
if bytes.Equal(acc.PubKey.Bytes(), account.PubKey.Bytes()) {
return fmt.Errorf("account with pubkey %s already exists", account.PubKey.String())
}
}
g.accounts = append(g.accounts, account)
return nil
}

func (g *Genesis) NewAccount(acc KeyringAccount) error {
if err := acc.ValidateBasic(); err != nil {
return err
}
if acc.Name != "" {
_, err := g.kr.Key(acc.Name)
if err == nil {
return fmt.Errorf("account with name %s already exists", acc.Name)
}
record, _, err := g.kr.NewMnemonic(acc.Name, keyring.English, "", "", hd.Secp256k1)
if err != nil {
return err
}
pk, err := record.GetPubKey()
if err != nil {
return fmt.Errorf("retrieving pubkey: %v", err)
}
acc.PubKey = pk
// check that the account does not already exist
if _, err := g.kr.Key(acc.Name); err == nil {
return fmt.Errorf("account with name %s already exists", acc.Name)
}
g.accounts = append(g.accounts, acc)

// generate the keys and add it to the genesis keyring
record, _, err := g.kr.NewMnemonic(acc.Name, keyring.English, "", "", hd.Secp256k1)
if err != nil {
return err
}

pubKey, err := record.GetPubKey()
if err != nil {
return err
}

account := Account{
PubKey: pubKey,
Balance: acc.InitialTokens,
}

g.accounts = append(g.accounts, account)
return nil
}

Expand All @@ -126,7 +143,7 @@ func (g *Genesis) AddValidator(val Validator) error {
}

// Add the validator's genesis account
if err := g.AddAccount(val.Account); err != nil {
if err := g.NewAccount(val.KeyringAccount); err != nil {
return err
}

Expand Down Expand Up @@ -162,7 +179,7 @@ func (g *Genesis) Export() (*coretypes.GenesisDoc, error) {
g.ConsensusParams,
g.ChainID,
gentxs,
g.Accounts(),
g.accounts,
g.genOps...,
)
}
Expand Down
Loading