Skip to content

Commit

Permalink
Merge #2699
Browse files Browse the repository at this point in the history
2699: [fix:`TestEpochJoinAndLeaveAN`] [fix:`TestEpochJoinAndLeaveVN`] Handle registering node without machine account  r=jordanschalm a=jordanschalm

Previously the epoch tests would pass in an argument representing a machine account key, even for nodes which had no machine account. This worked because the SDK accepted an empty account key and the argument was ignored.

An SDK API change caused this to no longer work, because the SDK will not accept empty account keys.

Instead, we handle the case of an empty machine account explicitly by passing in a nil optional for this parameter. This should address the 0% success rate for AN/VN tests, which do not use machine accounts:
* `TestEpochJoinAndLeaveAN`
* `TestEpochJoinAndLeaveVN`

Co-authored-by: Jordan Schalm <[email protected]>
  • Loading branch information
bors[bot] and jordanschalm authored Jun 30, 2022
2 parents 274c6e1 + 915a82d commit a984e61
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 25 deletions.
28 changes: 15 additions & 13 deletions integration/tests/epochs/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ import (

"github.com/onflow/cadence"
"github.com/onflow/flow-core-contracts/lib/go/templates"
sdk "github.com/onflow/flow-go-sdk"
sdkcrypto "github.com/onflow/flow-go-sdk/crypto"
"github.com/rs/zerolog"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

sdk "github.com/onflow/flow-go-sdk"
sdkcrypto "github.com/onflow/flow-go-sdk/crypto"

"github.com/onflow/flow-go/crypto"
"github.com/onflow/flow-go/engine/ghost/client"
"github.com/onflow/flow-go/integration/testnet"
Expand Down Expand Up @@ -138,16 +139,17 @@ func (s *Suite) TearDownTest() {
// StakedNodeOperationInfo struct contains all the node information needed to
// start a node after it is onboarded (staked and registered).
type StakedNodeOperationInfo struct {
NodeID flow.Identifier
Role flow.Role
StakingAccountAddress sdk.Address
FullAccountKey *sdk.AccountKey
StakingAccountKey sdkcrypto.PrivateKey
NetworkingKey sdkcrypto.PrivateKey
StakingKey sdkcrypto.PrivateKey
NodeID flow.Identifier
Role flow.Role
StakingAccountAddress sdk.Address
FullAccountKey *sdk.AccountKey
StakingAccountKey sdkcrypto.PrivateKey
NetworkingKey sdkcrypto.PrivateKey
StakingKey sdkcrypto.PrivateKey
// machine account info defined only for consensus/collection nodes
MachineAccountAddress sdk.Address
MachineAccountKey sdkcrypto.PrivateKey
MachineAccountPublicKey sdk.AccountKey
MachineAccountPublicKey *sdk.AccountKey
ContainerName string
}

Expand Down Expand Up @@ -283,7 +285,7 @@ func (s *Suite) generateAccountKeys(role flow.Role) (
networkingKey,
stakingKey,
machineAccountKey crypto.PrivateKey,
machineAccountPubKey sdk.AccountKey,
machineAccountPubKey *sdk.AccountKey,
) {
operatorAccountKey = unittest.PrivateKeyFixture(crypto.ECDSAP256, crypto.KeyGenSeedMinLenECDSAP256)
networkingKey = unittest.NetworkingPrivKeyFixture()
Expand All @@ -293,7 +295,7 @@ func (s *Suite) generateAccountKeys(role flow.Role) (
if role == flow.RoleConsensus || role == flow.RoleCollection {
machineAccountKey = unittest.PrivateKeyFixture(crypto.ECDSAP256, crypto.KeyGenSeedMinLenECDSAP256)

machineAccountPubKey = sdk.AccountKey{
machineAccountPubKey = &sdk.AccountKey{
PublicKey: machineAccountKey.PublicKey(),
SigAlgo: machineAccountKey.PublicKey().Algorithm(),
HashAlgo: bootstrap.DefaultMachineAccountHashAlgo,
Expand Down Expand Up @@ -359,7 +361,7 @@ func (s *Suite) SubmitStakingCollectionRegisterNodeTx(
networkingKey string,
stakingKey string,
amount string,
machineKey sdk.AccountKey,
machineKey *sdk.AccountKey,
) (*sdk.TransactionResult, sdk.Address, error) {
latestBlockID, err := s.client.GetLatestBlockID(ctx)
require.NoError(s.T(), err)
Expand Down
33 changes: 21 additions & 12 deletions integration/utils/transactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package utils

import (
"fmt"

"github.com/onflow/cadence"
"github.com/onflow/flow-core-contracts/lib/go/templates"
fttemplates "github.com/onflow/flow-ft/lib/go/templates"

sdk "github.com/onflow/flow-go-sdk"
"github.com/onflow/flow-go-sdk/crypto"
sdktemplates "github.com/onflow/flow-go-sdk/templates"
Expand Down Expand Up @@ -64,7 +66,7 @@ func MakeStakingCollectionRegisterNodeTx(
networkingKey string,
stakingKey string,
amount string,
machineKey sdk.AccountKey,
machineKey *sdk.AccountKey,
) (*sdk.Transaction, error) {
accountKey := stakingAccount.Keys[stakingAccountKeyID]
tx := sdk.NewTransaction().
Expand Down Expand Up @@ -111,17 +113,24 @@ func MakeStakingCollectionRegisterNodeTx(
return nil, err
}

publicKeys := make([]cadence.Value, 1)
publicKey, err := sdktemplates.AccountKeyToCadenceCryptoKey(&machineKey)
if err != nil {
return nil, err
}
publicKeys[0] = publicKey
publicKeysCDC := cadence.NewArray(publicKeys)

err = tx.AddArgument(cadence.NewOptional(publicKeysCDC))
if err != nil {
return nil, err
if machineKey != nil {
// for collection/consensus nodes, register the machine account key
publicKey, err := sdktemplates.AccountKeyToCadenceCryptoKey(machineKey)
if err != nil {
return nil, err
}
publicKeysCDC := cadence.NewArray([]cadence.Value{publicKey})

err = tx.AddArgument(cadence.NewOptional(publicKeysCDC))
if err != nil {
return nil, err
}
} else {
// for other nodes, pass nil to avoid registering any machine account key
err = tx.AddArgument(cadence.NewOptional(nil))
if err != nil {
return nil, err
}
}

err = tx.SignPayload(stakingAccount.Address, stakingAccountKeyID, stakingSigner)
Expand Down

0 comments on commit a984e61

Please sign in to comment.