Peer authentication sender factory #4414
Conversation
|
|
||
| func (pasf *peerAuthenticationSenderFactory) createMultikeyPeerAuthenticationSender() (*multikeyPeerAuthenticationSender, error) { | ||
| args := argMultikeyPeerAuthenticationSender{ | ||
| argBaseSender: argBaseSender{ |
There was a problem hiding this comment.
could have extracted a function createBaseSenderArgs and use it in the both functions
There was a problem hiding this comment.
not needed anymore with latest push
| return nil, fmt.Errorf("%w for peerAuthenticationSenderFactory, could not determine node's type", heartbeat.ErrInvalidConfiguration) | ||
| _, _, err = args.nodesCoordinator.GetValidatorWithPublicKey(pkBytes) | ||
| if err == nil { | ||
| return createRegularSender(args) |
There was a problem hiding this comment.
please add a check here for len(keysMap) and allow to create the regular peer only if that len is 0. Otherwise we should output error
…ests, partially integrated the sender parameters into heartbeat v2 components
factory/heartbeatV2Components.go
Outdated
| @@ -142,6 +143,9 @@ func (hcf *heartbeatV2ComponentsFactory) Create() (*heartbeatV2Components, error | |||
| HardforkTrigger: hcf.processComponents.HardforkTrigger(), | |||
| HardforkTimeBetweenSends: time.Second * time.Duration(cfg.HardforkTimeBetweenSendsInSec), | |||
| HardforkTriggerPubKey: hcf.coreComponents.HardforkTriggerPubKey(), | |||
| KeysHolder: &testscommon.KeysHolderStub{}, // TODO: use the real component once it is completely integrated | |||
There was a problem hiding this comment.
don't use mock / test packages in production code. Please create a disabled package somewhere, alongside with a disabledKeysHolder and use this instead.
There was a problem hiding this comment.
Agree, let's pass the real component here. Should be created & stored in crypto components
There was a problem hiding this comment.
pushed the integration into crypto components
cmd/node/config/config.toml
Outdated
| @@ -486,6 +486,10 @@ | |||
| MaxRoundsToKeepUnprocessedMiniBlocks = 300 # max number of rounds unprocessed miniblocks are kept in pool | |||
| MaxRoundsToKeepUnprocessedTransactions = 300 # max number of rounds unprocessed transactions are kept in pool | |||
|
|
|||
| [KeysHolderConfig] | |||
| IsMainMachine = false | |||
There was a problem hiding this comment.
why do we have a separate configuration? Couldn't this be determined based on the redundancy level?
There was a problem hiding this comment.
we have this in prefs.toml file
There was a problem hiding this comment.
and the MaxRoundsWithoutReceivedMessages is actually the RedundancyLevel value
factory/cryptoComponents.go
Outdated
| @@ -167,6 +174,19 @@ func (ccf *cryptoComponentsFactory) Create() (*cryptoComponents, error) { | |||
| return nil, err | |||
| } | |||
|
|
|||
| blsKeyGenerator := signing.NewKeyGenerator(mcl.NewSuiteBLS12()) | |||
There was a problem hiding this comment.
shouldn't it be created based on a config? similar to factory/cryptoComponents.getSuite()
cmd/node/config/config.toml
Outdated
| @@ -486,6 +486,10 @@ | |||
| MaxRoundsToKeepUnprocessedMiniBlocks = 300 # max number of rounds unprocessed miniblocks are kept in pool | |||
| MaxRoundsToKeepUnprocessedTransactions = 300 # max number of rounds unprocessed transactions are kept in pool | |||
|
|
|||
| [KeysHolderConfig] | |||
| IsMainMachine = false | |||
There was a problem hiding this comment.
we have this in prefs.toml file
cmd/node/config/config.toml
Outdated
| @@ -486,6 +486,10 @@ | |||
| MaxRoundsToKeepUnprocessedMiniBlocks = 300 # max number of rounds unprocessed miniblocks are kept in pool | |||
| MaxRoundsToKeepUnprocessedTransactions = 300 # max number of rounds unprocessed transactions are kept in pool | |||
|
|
|||
| [KeysHolderConfig] | |||
| IsMainMachine = false | |||
There was a problem hiding this comment.
and the MaxRoundsWithoutReceivedMessages is actually the RedundancyLevel value
factory/cryptoComponents.go
Outdated
| @@ -167,6 +174,19 @@ func (ccf *cryptoComponentsFactory) Create() (*cryptoComponents, error) { | |||
| return nil, err | |||
| } | |||
|
|
|||
| blsKeyGenerator := signing.NewKeyGenerator(mcl.NewSuiteBLS12()) | |||
There was a problem hiding this comment.
instead of this, we should use the blockSignKeyGen
| @@ -15,7 +15,7 @@ import ( | |||
| "github.com/ElrondNetwork/elrond-go/config" | |||
| ) | |||
|
|
|||
| const minRoundsWithoutReceivedMessages = 1 | |||
| const minRoundsWithoutReceivedMessages = -1 | |||
There was a problem hiding this comment.
we might refactor this in a future PR and re-check the logic.
Description of the reasoning behind the pull request (what feature was missing / how the problem was manifesting itself / what was the motive behind the refactoring)
Proposed Changes
Testing procedure