From 9a74e5feb9c9cb5d73969f4e6f11ff76f7dfec93 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Thu, 15 Dec 2022 16:55:48 -0800 Subject: [PATCH 01/21] Stabilizing consensus --- build/config/config1.json | 6 +-- build/config/config2.json | 6 +-- build/config/config3.json | 6 +-- build/config/config4.json | 6 +-- build/config/genesis.json | 38 ++++++------------ build/pkeys/val1.json | 2 +- build/pkeys/val2.json | 2 +- build/pkeys/val3.json | 2 +- build/pkeys/val4.json | 2 +- consensus/block.go | 23 ++++++++--- consensus/debugging.go | 8 ++-- consensus/doc/CHANGELOG.md | 1 - consensus/helpers.go | 21 ++++++---- consensus/hotstuff_handler.go | 20 +++------- consensus/hotstuff_leader.go | 59 +++++++++++++--------------- consensus/hotstuff_replica.go | 20 +++++----- consensus/pacemaker.go | 51 +++++++++++++----------- consensus/types/errors.go | 6 ++- p2p/addrbook_provider/persistence.go | 2 +- shared/node.go | 2 +- 20 files changed, 143 insertions(+), 140 deletions(-) diff --git a/build/config/config1.json b/build/config/config1.json index 359328cb5..395a25e47 100644 --- a/build/config/config1.json +++ b/build/config/config1.json @@ -1,7 +1,7 @@ { "base": { "root_directory": "/go/src/github.com/pocket-network", - "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4" + "private_key": "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a" }, "consensus": { "max_mempool_bytes": 500000000, @@ -10,7 +10,7 @@ "manual": true, "debug_time_between_steps_msec": 1000 }, - "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4" + "private_key": "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a" }, "utility": { "max_mempool_transaction_bytes": 1073741824, @@ -27,7 +27,7 @@ "consensus_port": 8080, "use_rain_tree": true, "is_empty_connection_type": false, - "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4" + "private_key": "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a" }, "telemetry": { "enabled": true, diff --git a/build/config/config2.json b/build/config/config2.json index 3b60ffa62..3f3d79536 100644 --- a/build/config/config2.json +++ b/build/config/config2.json @@ -1,7 +1,7 @@ { "base": { "root_directory": "/go/src/github.com/pocket-network", - "private_key": "5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb" + "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" }, "consensus": { "max_mempool_bytes": 500000000, @@ -10,7 +10,7 @@ "manual": true, "debug_time_between_steps_msec": 1000 }, - "private_key": "5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb" + "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" }, "utility": { "max_mempool_transaction_bytes": 1073741824, @@ -27,7 +27,7 @@ "consensus_port": 8080, "use_rain_tree": true, "is_empty_connection_type": false, - "private_key": "5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb" + "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" }, "telemetry": { "enabled": true, diff --git a/build/config/config3.json b/build/config/config3.json index 54140b350..d847f40eb 100644 --- a/build/config/config3.json +++ b/build/config/config3.json @@ -1,7 +1,7 @@ { "base": { "root_directory": "/go/src/github.com/pocket-network", - "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" + "private_key": "5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb" }, "consensus": { "max_mempool_bytes": 500000000, @@ -10,7 +10,7 @@ "manual": true, "debug_time_between_steps_msec": 1000 }, - "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" + "private_key": "5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb" }, "utility": { "max_mempool_transaction_bytes": 1073741824, @@ -27,7 +27,7 @@ "consensus_port": 8080, "use_rain_tree": true, "is_empty_connection_type": false, - "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" + "private_key": "5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb" }, "telemetry": { "enabled": true, diff --git a/build/config/config4.json b/build/config/config4.json index 333ebad2e..bf402b135 100644 --- a/build/config/config4.json +++ b/build/config/config4.json @@ -1,7 +1,7 @@ { "base": { "root_directory": "/go/src/github.com/pocket-network", - "private_key": "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a" + "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4" }, "consensus": { "max_mempool_bytes": 500000000, @@ -10,7 +10,7 @@ "manual": true, "debug_time_between_steps_msec": 1000 }, - "private_key": "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a" + "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4" }, "utility": { "max_mempool_transaction_bytes": 1073741824, @@ -27,7 +27,7 @@ "consensus_port": 8080, "use_rain_tree": true, "is_empty_connection_type": false, - "private_key": "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a" + "private_key": "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4" }, "telemetry": { "enabled": true, diff --git a/build/config/genesis.json b/build/config/genesis.json index ad0784df5..f5b2fa32d 100755 --- a/build/config/genesis.json +++ b/build/config/genesis.json @@ -58,63 +58,51 @@ ], "validators": [ { - "address": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", - "public_key": "b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", + "address": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", + "public_key": "53ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", "chains": null, "generic_param": "node1.consensus:8080", "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, - "output": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", + "output": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", "actor_type": 3 }, { - "address": "67eb3f0a50ae459fecf666be0e93176e92441317", - "public_key": "c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb", + "address": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", + "public_key": "a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2", "chains": null, "generic_param": "node2.consensus:8080", "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, - "output": "67eb3f0a50ae459fecf666be0e93176e92441317", + "output": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", "actor_type": 3 }, { - "address": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", - "public_key": "a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2", + "address": "67eb3f0a50ae459fecf666be0e93176e92441317", + "public_key": "c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb", "chains": null, "generic_param": "node3.consensus:8080", "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, - "output": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", + "output": "67eb3f0a50ae459fecf666be0e93176e92441317", "actor_type": 3 }, { - "address": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", - "public_key": "53ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", + "address": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", + "public_key": "b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", "chains": null, "generic_param": "node4.consensus:8080", "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, - "output": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", + "output": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", "actor_type": 3 } ], - "applications": [ - { - "address": "88a792b7aca673620132ef01f50e62caa58eca83", - "public_key": "5f78658599943dc3e623539ce0b3c9fe4e192034a1e3fef308bc9f96915754e0", - "chains": ["0001"], - "generic_param": "1000000", - "staked_amount": "1000000000000", - "paused_height": -1, - "unstaking_height": -1, - "output": "88a792b7aca673620132ef01f50e62caa58eca83", - "actor_type": 0 - } - ], + "applications": [], "service_nodes": [ { "address": "43d9ea9d9ad9c58bb96ec41340f83cb2cabb6496", diff --git a/build/pkeys/val1.json b/build/pkeys/val1.json index 94149390a..4ba0a6013 100644 --- a/build/pkeys/val1.json +++ b/build/pkeys/val1.json @@ -1 +1 @@ -"6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4" +"c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a" diff --git a/build/pkeys/val2.json b/build/pkeys/val2.json index fb3fd2fd9..d00e383c0 100644 --- a/build/pkeys/val2.json +++ b/build/pkeys/val2.json @@ -1 +1 @@ -"5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb" +"b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" diff --git a/build/pkeys/val3.json b/build/pkeys/val3.json index d00e383c0..fb3fd2fd9 100644 --- a/build/pkeys/val3.json +++ b/build/pkeys/val3.json @@ -1 +1 @@ -"b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" +"5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb" diff --git a/build/pkeys/val4.json b/build/pkeys/val4.json index 4ba0a6013..94149390a 100644 --- a/build/pkeys/val4.json +++ b/build/pkeys/val4.json @@ -1 +1 @@ -"c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a" +"6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4" diff --git a/consensus/block.go b/consensus/block.go index 75b62ab80..1b2fd9ce4 100644 --- a/consensus/block.go +++ b/consensus/block.go @@ -1,6 +1,7 @@ package consensus import ( + "fmt" "log" "unsafe" @@ -26,13 +27,19 @@ func (m *consensusModule) commitBlock(block *typesCons.Block) error { // TODO: Add unit tests specific to block validation // IMPROVE: (olshansky) rename to provide clarity of operation. ValidateBasic() is typically a stateless check not stateful -func (m *consensusModule) validateBlockBasic(block *typesCons.Block) error { - if block == nil && m.step != NewRound { - return typesCons.ErrNilBlock +func (m *consensusModule) validateMessageBlock(msg *typesCons.HotstuffMessage) error { + block := msg.GetBlock() + step := msg.GetStep() + + if block == nil { + if step != NewRound { + return fmt.Errorf("validateBlockBasic failed - block is nil during step %s", typesCons.StepToString[m.step]) + } + return nil } - if block != nil && m.step == NewRound { - return typesCons.ErrBlockExists + if block != nil && step == NewRound { + return fmt.Errorf("validateBlockBasic failed - block is not nil during step %s", typesCons.StepToString[m.step]) } if block != nil && unsafe.Sizeof(*block) > uintptr(m.consGenesis.GetMaxBlockBytes()) { @@ -42,10 +49,14 @@ func (m *consensusModule) validateBlockBasic(block *typesCons.Block) error { // If the current block being processed (i.e. voted on) by consensus is non nil, we need to make // sure that the data (height, round, step, txs, etc) is the same before we start validating the signatures if m.block != nil { + if m.block.BlockHeader.Hash != block.BlockHeader.Hash { + return fmt.Errorf("validateBlockBasic failed - block hash is not the same as the current block being processed by consensus") + } + // DISCUSS: The only difference between blocks from one step to another is the QC, so we need // to determine where/how to validate this if protoHash(m.block) != protoHash(block) { - log.Println("[TECHDEBT][ERROR] The block being processed is not the same as that received by the consensus module ") + log.Println("[TECHDEBT] validateBlockBasic warning - block hash is the same but serialization is not") } } diff --git a/consensus/debugging.go b/consensus/debugging.go index f44b29189..f49fa924e 100644 --- a/consensus/debugging.go +++ b/consensus/debugging.go @@ -8,6 +8,9 @@ import ( ) func (m *consensusModule) HandleDebugMessage(debugMessage *messaging.DebugMessage) error { + m.m.Lock() + defer m.m.Unlock() + switch debugMessage.Action { case messaging.DebugMessageAction_DEBUG_CONSENSUS_RESET_TO_GENESIS: m.resetToGenesis(debugMessage) @@ -24,12 +27,11 @@ func (m *consensusModule) HandleDebugMessage(debugMessage *messaging.DebugMessag } func (m *consensusModule) GetNodeState() typesCons.ConsensusNodeState { - m.m.RLock() - defer m.m.RUnlock() leaderId := typesCons.NodeId(0) if m.leaderId != nil { leaderId = *m.leaderId } + return typesCons.ConsensusNodeState{ NodeId: m.nodeId, Height: m.height, @@ -67,7 +69,7 @@ func (m *consensusModule) triggerNextView(_ *messaging.DebugMessage) { if currentHeight == 0 || (currentStep == Decide && m.paceMaker.IsManualMode()) { m.paceMaker.NewHeight() } else { - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("manual trigger") } if m.paceMaker.IsManualMode() { diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index b4211d402..53ff034ed 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -7,7 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - ## [0.0.0.13] - 2022-12-14 - Consolidated number of validators in tests in a single constant: `numValidators` diff --git a/consensus/helpers.go b/consensus/helpers.go index 8ef8fc6e6..1d26f9159 100644 --- a/consensus/helpers.go +++ b/consensus/helpers.go @@ -137,35 +137,42 @@ func protoHash(m proto.Message) string { /*** P2P Helpers ***/ -func (m *consensusModule) sendToNode(msg *typesCons.HotstuffMessage) { - // TODO(olshansky): This can happen due to a race condition with the pacemaker. +func (m *consensusModule) sendToLeader(msg *typesCons.HotstuffMessage) { + m.nodeLog(typesCons.SendingMessage(msg, *m.leaderId)) + + // TODO: This can happen due to a race condition with the pacemaker. if m.leaderId == nil { m.nodeLogError(typesCons.ErrNilLeaderId.Error(), nil) return } - m.nodeLog(typesCons.SendingMessage(msg, *m.leaderId)) anyConsensusMessage, err := codec.GetCodec().ToAny(msg) if err != nil { m.nodeLogError(typesCons.ErrCreateConsensusMessage.Error(), err) return } + if err := m.GetBus().GetP2PModule().Send(cryptoPocket.AddressFromString(m.idToValAddrMap[*m.leaderId]), anyConsensusMessage); err != nil { m.nodeLogError(typesCons.ErrSendMessage.Error(), err) return } } -func (m *consensusModule) broadcastToNodes(msg *typesCons.HotstuffMessage) { +// Star-like "broadcast" - send to all nodes directly +func (m *consensusModule) broadcastToValidators(msg *typesCons.HotstuffMessage) { m.nodeLog(typesCons.BroadcastingMessage(msg)) + anyConsensusMessage, err := codec.GetCodec().ToAny(msg) if err != nil { m.nodeLogError(typesCons.ErrCreateConsensusMessage.Error(), err) return } - if err := m.GetBus().GetP2PModule().Broadcast(anyConsensusMessage); err != nil { - m.nodeLogError(typesCons.ErrBroadcastMessage.Error(), err) - return + + for _, val := range m.validatorMap { + // fmt.Println("OLSH", addr) + if err := m.GetBus().GetP2PModule().Send(cryptoPocket.AddressFromString(val.GetAddress()), anyConsensusMessage); err != nil { + m.nodeLogError(typesCons.ErrBroadcastMessage.Error(), err) + } } } diff --git a/consensus/hotstuff_handler.go b/consensus/hotstuff_handler.go index 51db9f74e..9a2e74095 100644 --- a/consensus/hotstuff_handler.go +++ b/consensus/hotstuff_handler.go @@ -19,24 +19,23 @@ func (m *consensusModule) handleHotstuffMessage(msg *typesCons.HotstuffMessage) step := msg.GetStep() // Pacemaker - Liveness & safety checks - if err := m.paceMaker.ValidateMessage(msg); err != nil { - if m.shouldHandleHotstuffMessage(step) { - m.nodeLog(typesCons.WarnDiscardHotstuffMessage(msg, err.Error())) - return err - } - return nil + if shouldHandle, err := m.paceMaker.ShouldHandleMessage(msg); !shouldHandle { + return err } + // Elect a leader for the current round if needed if m.shouldElectNextLeader() { if err := m.electNextLeader(msg); err != nil { return err } } - // Hotstuff - Handle message + // Hotstuff - Handle message as a replica if m.isReplica() { replicaHandlers[step](m, msg) } + + // Hotstuff - Handle message as a leader // Note that the leader also acts as a replica, but this logic is implemented in the underlying code. leaderHandlers[step](m, msg) @@ -47,10 +46,3 @@ func (m *consensusModule) shouldElectNextLeader() bool { // Execute leader election if there is no leader and we are in a new round return m.step == NewRound && m.leaderId == nil } - -func (m *consensusModule) shouldHandleHotstuffMessage(step typesCons.HotstuffStep) bool { - // If a replica is not a leader for this round, but has already determined a leader, - // and continues to receive NewRound messages, we avoid logging the "message discard" - // because it creates unnecessary spam. - return !(m.leaderId != nil && !m.isLeader() && step == NewRound) -} diff --git a/consensus/hotstuff_leader.go b/consensus/hotstuff_leader.go index ecefe05ec..48924f816 100644 --- a/consensus/hotstuff_leader.go +++ b/consensus/hotstuff_leader.go @@ -57,7 +57,7 @@ func (handler *HotstuffLeaderMessageHandler) HandleNewRoundMessage(m *consensusM block, err := m.prepareAndApplyBlock(highPrepareQC) if err != nil { m.nodeLogError(typesCons.ErrPrepareBlock.Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to prepare & apply block") return } m.block = block @@ -66,7 +66,7 @@ func (handler *HotstuffLeaderMessageHandler) HandleNewRoundMessage(m *consensusM // TODO: Do we need to call `validateProposal` here similar to how replicas does it if err := m.applyBlock(highPrepareQC.Block); err != nil { m.nodeLogError(typesCons.ErrApplyBlock.Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to apply block") return } m.block = highPrepareQC.Block @@ -78,10 +78,10 @@ func (handler *HotstuffLeaderMessageHandler) HandleNewRoundMessage(m *consensusM prepareProposeMessage, err := CreateProposeMessage(m.height, m.round, Prepare, m.block, highPrepareQC) if err != nil { m.nodeLogError(typesCons.ErrCreateProposeMessage(Prepare).Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to create propose message") return } - m.broadcastToNodes(prepareProposeMessage) + m.broadcastToValidators(prepareProposeMessage) // Leader also acts like a replica prepareVoteMessage, err := CreateVoteMessage(m.height, m.round, Prepare, m.block, m.privateKey) @@ -89,7 +89,7 @@ func (handler *HotstuffLeaderMessageHandler) HandleNewRoundMessage(m *consensusM m.nodeLogError(typesCons.ErrCreateVoteMessage(Prepare).Error(), err) return } - m.sendToNode(prepareVoteMessage) + m.sendToLeader(prepareVoteMessage) } /*** PreCommit Step ***/ @@ -122,10 +122,10 @@ func (handler *HotstuffLeaderMessageHandler) HandlePrepareMessage(m *consensusMo preCommitProposeMessage, err := CreateProposeMessage(m.height, m.round, PreCommit, m.block, prepareQC) if err != nil { m.nodeLogError(typesCons.ErrCreateProposeMessage(PreCommit).Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to create propose message") return } - m.broadcastToNodes(preCommitProposeMessage) + m.broadcastToValidators(preCommitProposeMessage) // Leader also acts like a replica precommitVoteMessage, err := CreateVoteMessage(m.height, m.round, PreCommit, m.block, m.privateKey) @@ -133,7 +133,7 @@ func (handler *HotstuffLeaderMessageHandler) HandlePrepareMessage(m *consensusMo m.nodeLogError(typesCons.ErrCreateVoteMessage(PreCommit).Error(), err) return } - m.sendToNode(precommitVoteMessage) + m.sendToLeader(precommitVoteMessage) } /*** Commit Step ***/ @@ -166,10 +166,10 @@ func (handler *HotstuffLeaderMessageHandler) HandlePrecommitMessage(m *consensus commitProposeMessage, err := CreateProposeMessage(m.height, m.round, Commit, m.block, preCommitQC) if err != nil { m.nodeLogError(typesCons.ErrCreateProposeMessage(Commit).Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to create propose message") return } - m.broadcastToNodes(commitProposeMessage) + m.broadcastToValidators(commitProposeMessage) // Leader also acts like a replica commitVoteMessage, err := CreateVoteMessage(m.height, m.round, Commit, m.block, m.privateKey) @@ -177,7 +177,7 @@ func (handler *HotstuffLeaderMessageHandler) HandlePrecommitMessage(m *consensus m.nodeLogError(typesCons.ErrCreateVoteMessage(Commit).Error(), err) return } - m.sendToNode(commitVoteMessage) + m.sendToLeader(commitVoteMessage) } /*** Decide Step ***/ @@ -209,14 +209,14 @@ func (handler *HotstuffLeaderMessageHandler) HandleCommitMessage(m *consensusMod decideProposeMessage, err := CreateProposeMessage(m.height, m.round, Decide, m.block, commitQC) if err != nil { m.nodeLogError(typesCons.ErrCreateProposeMessage(Decide).Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to create propose message") return } - m.broadcastToNodes(decideProposeMessage) + m.broadcastToValidators(decideProposeMessage) if err := m.commitBlock(m.block); err != nil { m.nodeLogError(typesCons.ErrCommitBlock.Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to commit block") return } @@ -245,17 +245,20 @@ func (handler *HotstuffLeaderMessageHandler) HandleDecideMessage(m *consensusMod func (handler *HotstuffLeaderMessageHandler) anteHandle(m *consensusModule, msg *typesCons.HotstuffMessage) error { // Basic block metadata validation - if err := m.validateBlockBasic(msg.GetBlock()); err != nil { + if err := m.validateMessageBlock(msg); err != nil { return err } // Discard messages with invalid partial signatures before storing it in the leader's consensus mempool - if err := m.validatePartialSignature(msg); err != nil { + if err := m.validateMessageSignature(msg); err != nil { + return err + } + + // Index the hotstuff message in the consensus mempool + if err := m.indexHotstuffMessage(msg); err != nil { return err } - // TECHDEBT: Until we integrate with the real mempool, this is a makeshift solution - m.tempIndexHotstuffMessage(msg) return nil } @@ -272,16 +275,7 @@ func (handler *HotstuffLeaderMessageHandler) emitTelemetryEvent(m *consensusModu ) } -// ValidateBasic general validation checks that apply to every HotstuffLeaderMessage -func (handler *HotstuffLeaderMessageHandler) validateBasic(m *consensusModule, msg *typesCons.HotstuffMessage) error { - // Discard messages with invalid partial signatures before storing it in the leader's consensus mempool - if err := m.validatePartialSignature(msg); err != nil { - return err - } - return nil -} - -func (m *consensusModule) validatePartialSignature(msg *typesCons.HotstuffMessage) error { +func (m *consensusModule) validateMessageSignature(msg *typesCons.HotstuffMessage) error { if msg.GetStep() == NewRound { m.nodeLog(typesCons.ErrUnnecessaryPartialSigForNewRound.Error()) return nil @@ -315,20 +309,21 @@ func (m *consensusModule) validatePartialSignature(msg *typesCons.HotstuffMessag address, m.valAddrToIdMap[address], msg, pubKey) } -// TODO: This is just a placeholder at the moment for indexing hotstuff messages ONLY. +// TODO(#388): Utilize the shared mempool implementation for consensus messages. // // It doesn't actually work because SizeOf returns the size of the map pointer, // and does not recursively determine the size of all the underlying elements // Add proper tests and implementation once the mempool is implemented. -func (m *consensusModule) tempIndexHotstuffMessage(msg *typesCons.HotstuffMessage) { +func (m *consensusModule) indexHotstuffMessage(msg *typesCons.HotstuffMessage) error { if m.consCfg.GetMaxMempoolBytes() < uint64(unsafe.Sizeof(m.messagePool)) { - m.nodeLogError(typesCons.DisregardHotstuffMessage, typesCons.ErrConsensusMempoolFull) - return + return typesCons.ErrConsensusMempoolFull } // Only the leader needs to aggregate consensus related messages. step := msg.GetStep() m.messagePool[step] = append(m.messagePool[step], msg) + + return nil } // This is a helper function intended to be called by a leader/validator during a view change diff --git a/consensus/hotstuff_replica.go b/consensus/hotstuff_replica.go index db4561fd5..cc485f141 100644 --- a/consensus/hotstuff_replica.go +++ b/consensus/hotstuff_replica.go @@ -55,14 +55,14 @@ func (handler *HotstuffReplicaMessageHandler) HandlePrepareMessage(m *consensusM if err := m.validateProposal(msg); err != nil { m.nodeLogError(fmt.Sprintf("Invalid proposal in %s message", Prepare), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("invalid proposal") return } block := msg.GetBlock() if err := m.applyBlock(block); err != nil { m.nodeLogError(typesCons.ErrApplyBlock.Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to apply block") return } m.block = block @@ -73,7 +73,7 @@ func (handler *HotstuffReplicaMessageHandler) HandlePrepareMessage(m *consensusM m.nodeLogError(typesCons.ErrCreateVoteMessage(Prepare).Error(), err) return // Not interrupting the round because liveness could continue with one failed vote } - m.sendToNode(prepareVoteMessage) + m.sendToLeader(prepareVoteMessage) } /*** PreCommit Step ***/ @@ -90,7 +90,7 @@ func (handler *HotstuffReplicaMessageHandler) HandlePrecommitMessage(m *consensu quorumCert := msg.GetQuorumCertificate() if err := m.validateQuorumCertificate(quorumCert); err != nil { m.nodeLogError(typesCons.ErrQCInvalid(PreCommit).Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("invalid quorum certificate") return } @@ -102,7 +102,7 @@ func (handler *HotstuffReplicaMessageHandler) HandlePrecommitMessage(m *consensu m.nodeLogError(typesCons.ErrCreateVoteMessage(PreCommit).Error(), err) return // Not interrupting the round because liveness could continue with one failed vote } - m.sendToNode(preCommitVoteMessage) + m.sendToLeader(preCommitVoteMessage) } /*** Commit Step ***/ @@ -119,7 +119,7 @@ func (handler *HotstuffReplicaMessageHandler) HandleCommitMessage(m *consensusMo quorumCert := msg.GetQuorumCertificate() if err := m.validateQuorumCertificate(quorumCert); err != nil { m.nodeLogError(typesCons.ErrQCInvalid(Commit).Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("invalid quorum certificate") return } @@ -131,7 +131,7 @@ func (handler *HotstuffReplicaMessageHandler) HandleCommitMessage(m *consensusMo m.nodeLogError(typesCons.ErrCreateVoteMessage(Commit).Error(), err) return // Not interrupting the round because liveness could continue with one failed vote } - m.sendToNode(commitVoteMessage) + m.sendToLeader(commitVoteMessage) } /*** Decide Step ***/ @@ -148,13 +148,13 @@ func (handler *HotstuffReplicaMessageHandler) HandleDecideMessage(m *consensusMo quorumCert := msg.GetQuorumCertificate() if err := m.validateQuorumCertificate(quorumCert); err != nil { m.nodeLogError(typesCons.ErrQCInvalid(Decide).Error(), err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("invalid quorum certificate") return } if err := m.commitBlock(m.block); err != nil { m.nodeLogError("Could not commit block", err) - m.paceMaker.InterruptRound() + m.paceMaker.InterruptRound("failed to commit block") return } @@ -164,7 +164,7 @@ func (handler *HotstuffReplicaMessageHandler) HandleDecideMessage(m *consensusMo // anteHandle is the handler called on every replica message before specific handler func (handler *HotstuffReplicaMessageHandler) anteHandle(m *consensusModule, msg *typesCons.HotstuffMessage) error { // Basic block metadata validation - if err := m.validateBlockBasic(msg.GetBlock()); err != nil { + if err := m.validateMessageBlock(msg); err != nil { return err } diff --git a/consensus/pacemaker.go b/consensus/pacemaker.go index bb7b3c3ca..f0752b83a 100644 --- a/consensus/pacemaker.go +++ b/consensus/pacemaker.go @@ -24,10 +24,10 @@ type Pacemaker interface { // for the height/round/step/etc, and interface with the module that way. SetConsensusModule(module *consensusModule) - ValidateMessage(message *typesCons.HotstuffMessage) error + ShouldHandleMessage(message *typesCons.HotstuffMessage) (bool, error) RestartTimer() NewHeight() - InterruptRound() + InterruptRound(reason string) } var ( @@ -108,7 +108,7 @@ func (m *paceMaker) GetBus() modules.Bus { func (*paceMaker) ValidateConfig(cfg modules.Config) error { if _, ok := cfg.GetConsensusConfig().(HasPacemakerConfig); !ok { - return fmt.Errorf("cannot cast to PacemakeredConsensus") + return fmt.Errorf("cannot cast to PacemakerConfig") } return nil } @@ -117,36 +117,45 @@ func (m *paceMaker) SetConsensusModule(c *consensusModule) { m.consensusMod = c } -func (p *paceMaker) ValidateMessage(m *typesCons.HotstuffMessage) error { +func (p *paceMaker) ShouldHandleMessage(m *typesCons.HotstuffMessage) (bool, error) { currentHeight := p.consensusMod.height currentRound := p.consensusMod.round + // Consensus message is from the past if m.Height < currentHeight { - return typesCons.ErrPacemakerUnexpectedMessageHeight(typesCons.ErrOlderMessage, currentHeight, m.Height) + // TODO: Noisy log - make it a DEBUG + // p.consensusMod.nodeLog(typesCons.ErrPacemakerUnexpectedMessageHeight(typesCons.ErrOlderMessage, currentHeight, m.Height).Error()) + return false, nil } // Current node is out of sync + // TODO: Need to restart state sync or be in state sync mode right now if m.Height > currentHeight { - // TODO(design): Need to restart state sync - return typesCons.ErrPacemakerUnexpectedMessageHeight(typesCons.ErrFutureMessage, currentHeight, m.Height) + // TODO: Noisy log - make it a DEBUG + // p.consensusMod.nodeLog(typesCons.ErrPacemakerUnexpectedMessageHeight(typesCons.ErrFutureMessage, currentHeight, m.Height).Error()) + return false, nil } // Do not handle messages if it is a self proposal + // TODO(olshansky): This code branch is a result of the optimization in the leader + // handlers. Since the leader also acts as a replica but doesn't use the replica's + // handlers given the current implementation, it is safe to drop proposal that the leader made to itself. if p.consensusMod.isLeader() && m.Type == Propose && m.Step != NewRound { - // TODO(olshansky): This code branch is a result of the optimization in the leader - // handlers. Since the leader also acts as a replica but doesn't use the replica's - // handlers given the current implementation, it is safe to drop proposal that the leader made to itself. - return typesCons.ErrSelfProposal + // TODO: Noisy log - make it a DEBUG + // p.consensusMod.nodeLog(typesCons.ErrSelfProposal.Error()) + return false, nil } // Message is from the past if m.Round < currentRound || (m.Round == currentRound && m.Step < p.consensusMod.step) { - return typesCons.ErrPacemakerUnexpectedMessageStepRound(typesCons.ErrOlderStepRound, p.consensusMod.step, currentRound, m) + // TODO: Noisy log - make it a DEBUG + // p.consensusMod.nodeLog(typesCons.ErrPacemakerUnexpectedMessageStepRound(typesCons.ErrOlderStepRound, p.consensusMod.step, currentRound, m).Error()) + return false, nil } // Everything checks out! if m.Height == currentHeight && m.Step == p.consensusMod.step && m.Round == currentRound { - return nil + return true, nil } // Pacemaker catch up! Node is synched to the right height, but on a previous step/round so we just jump to the latest state. @@ -161,19 +170,18 @@ func (p *paceMaker) ValidateMessage(m *typesCons.HotstuffMessage) error { p.consensusMod.electNextLeader(m) } - return nil + return true, nil } - return typesCons.ErrUnexpectedPacemakerCase + return false, typesCons.ErrUnexpectedPacemakerCase } func (p *paceMaker) RestartTimer() { + // NOTE: Not deferring a cancel call because this function is asynchronous. if p.stepCancelFunc != nil { p.stepCancelFunc() } - // NOTE: Not defering a cancel call because this function is asynchronous. - stepTimeout := p.getStepTimeout(p.consensusMod.round) clock := p.GetBus().GetRuntimeMgr().GetClock() @@ -185,8 +193,7 @@ func (p *paceMaker) RestartTimer() { select { case <-ctx.Done(): if ctx.Err() == context.DeadlineExceeded { - p.consensusMod.nodeLog(typesCons.PacemakerTimeout(p.consensusMod.CurrentHeight(), p.consensusMod.step, p.consensusMod.round)) - p.InterruptRound() + p.InterruptRound("timeout") } case <-clock.After(stepTimeout + 30*timePkg.Millisecond): // Adding 30ms to the context timeout to avoid race condition. return @@ -194,8 +201,8 @@ func (p *paceMaker) RestartTimer() { }() } -func (p *paceMaker) InterruptRound() { - p.consensusMod.nodeLog(typesCons.PacemakerInterrupt(p.consensusMod.CurrentHeight(), p.consensusMod.step, p.consensusMod.round)) +func (p *paceMaker) InterruptRound(reason string) { + p.consensusMod.nodeLog(typesCons.PacemakerInterrupt(reason, p.consensusMod.CurrentHeight(), p.consensusMod.step, p.consensusMod.round)) p.consensusMod.round++ p.startNextView(p.consensusMod.highPrepareQC, false) @@ -254,7 +261,7 @@ func (p *paceMaker) startNextView(qc *typesCons.QuorumCertificate, forceNextView } p.RestartTimer() - p.consensusMod.broadcastToNodes(hotstuffMessage) + p.consensusMod.broadcastToValidators(hotstuffMessage) } // TODO(olshansky): Increase timeout using exponential backoff. diff --git a/consensus/types/errors.go b/consensus/types/errors.go index 351f875fa..3f6f9c855 100644 --- a/consensus/types/errors.go +++ b/consensus/types/errors.go @@ -1,5 +1,7 @@ package types +// TECHDEBT: Avoid having a centralized file for all errors (harder to maintain and identify). + import ( "encoding/base64" "errors" @@ -35,8 +37,8 @@ func init() { } } -func PacemakerInterrupt(height uint64, step HotstuffStep, round uint64) string { - return fmt.Sprintf("INTERRUPT at (height, step, round): (%d, %s, %d)!", height, StepToString[step], round) +func PacemakerInterrupt(reason string, height uint64, step HotstuffStep, round uint64) string { + return fmt.Sprintf("INTERRUPT due to %s at (height, step, round): (%d, %s, %d)!", reason, height, StepToString[step], round) } func PacemakerTimeout(height uint64, step HotstuffStep, round uint64) string { diff --git a/p2p/addrbook_provider/persistence.go b/p2p/addrbook_provider/persistence.go index eee396d5d..6d62fcc98 100644 --- a/p2p/addrbook_provider/persistence.go +++ b/p2p/addrbook_provider/persistence.go @@ -76,7 +76,7 @@ func (pabp *persistenceAddrBookProvider) ValidatorMapToAddrBook(validators map[s for _, v := range validators { networkPeer, err := pabp.ValidatorToNetworkPeer(v) if err != nil { - log.Println("[WARN] Error connecting to validator: ", err) + log.Println("[WARN] Error connecting to validator:", err) continue } book = append(book, networkPeer) diff --git a/shared/node.go b/shared/node.go index 3b4dac2a7..6c1bf1dda 100644 --- a/shared/node.go +++ b/shared/node.go @@ -133,7 +133,7 @@ func (node *Node) Start() error { for { event := node.GetBus().GetBusEvent() if err := node.handleEvent(event); err != nil { - log.Println("Error handling event: ", err) + log.Println("Error handling event:", err) } } } From d8ad6e872705ac71702909020f20ba7f45ae1282 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Thu, 22 Dec 2022 15:07:23 -0500 Subject: [PATCH 02/21] Handling concurrent reads and existing write contexts --- consensus/leader_election/module.go | 13 ++++++++----- consensus/types/errors.go | 9 +++++++-- p2p/raintree/network.go | 14 +++++++++++--- persistence/context.go | 2 +- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/consensus/leader_election/module.go b/consensus/leader_election/module.go index 8ab583121..c3178b4cd 100644 --- a/consensus/leader_election/module.go +++ b/consensus/leader_election/module.go @@ -63,15 +63,18 @@ func (m *leaderElectionModule) ElectNextLeader(message *typesCons.HotstuffMessag } func (m *leaderElectionModule) electNextLeaderDeterministicRoundRobin(message *typesCons.HotstuffMessage) (typesCons.NodeId, error) { - value := int64(message.Height) + int64(message.Round) + int64(message.Step) - 1 - - uCtx, err := m.GetBus().GetUtilityModule().NewContext(int64(message.Height)) + height := int64(message.Height) + readCtx, err := m.GetBus().GetPersistenceModule().NewReadContext(height) if err != nil { return typesCons.NodeId(0), err } - vals, err := uCtx.GetPersistenceContext().GetAllValidators(int64(message.Height)) + vals, err := readCtx.GetAllValidators(height) if err != nil { return typesCons.NodeId(0), err } - return typesCons.NodeId(value%int64(len(vals)) + 1), nil + + value := int64(message.Height) + int64(message.Round) + int64(message.Step) - 1 + numVals := int64(len(vals)) + + return typesCons.NodeId(value%numVals + 1), nil } diff --git a/consensus/types/errors.go b/consensus/types/errors.go index 3f6f9c855..aff764c76 100644 --- a/consensus/types/errors.go +++ b/consensus/types/errors.go @@ -37,8 +37,13 @@ func init() { } } +// TODO(#288): Improve all of this logging: +// 1. Replace `fmt.Sprintf` with `log.Printf` (or similar) +// 2. Add the appropriate log level (warn, debug, etc...) where appropriate +// 3. Remove this file and move log related text into place (easier to maintain, debug, understand, etc.) + func PacemakerInterrupt(reason string, height uint64, step HotstuffStep, round uint64) string { - return fmt.Sprintf("INTERRUPT due to %s at (height, step, round): (%d, %s, %d)!", reason, height, StepToString[step], round) + return fmt.Sprintf("INTERRUPT at (height, step, round): (%d, %s, %d)! Reason: %s", height, StepToString[step], round, reason) } func PacemakerTimeout(height uint64, step HotstuffStep, round uint64) string { @@ -106,7 +111,7 @@ func DebugTogglePacemakerManualMode(mode string) string { } func DebugNodeState(state ConsensusNodeState) string { - return fmt.Sprintf("[DEBUG] NODE STATE: Node %d is at (Height, Step, Round): (%d, %d, %d)\n", state.NodeId, state.Height, state.Step, state.Round) + return fmt.Sprintf("[DEBUG] Node %d is at (Height, Step, Round): (%d, %d, %d)\n", state.NodeId, state.Height, state.Step, state.Round) } func DebugHandlingHotstuffMessage(msg *HotstuffMessage) string { diff --git a/p2p/raintree/network.go b/p2p/raintree/network.go index 2251d282a..2c7c3958c 100644 --- a/p2p/raintree/network.go +++ b/p2p/raintree/network.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "math/rand" + "sync" "time" "github.com/pokt-network/pocket/p2p/addrbook_provider" @@ -31,7 +32,8 @@ type rainTreeNetwork struct { // TODO(#388): generalize to use the shared `FIFOMempool` type (in `utility/types/mempool.go` at the time of writing) in here as well for this. nonceSet map[uint64]struct{} nonceList []uint64 - mampoolMaxNonces uint64 + mempoolMaxNonces uint64 + mempoolLock sync.Mutex } func NewRainTreeNetworkWithAddrBook(addr cryptoPocket.Address, addrBook typesP2P.AddrBook, p2pCfg modules.P2PConfig) typesP2P.Network { @@ -46,7 +48,7 @@ func NewRainTreeNetworkWithAddrBook(addr cryptoPocket.Address, addrBook typesP2P peersManager: pm, nonceSet: make(map[uint64]struct{}), nonceList: make([]uint64, 0, mempoolMaxNonces), - mampoolMaxNonces: mempoolMaxNonces, + mempoolMaxNonces: mempoolMaxNonces, } return typesP2P.Network(n) @@ -208,9 +210,15 @@ func (n *rainTreeNetwork) HandleNetworkData(data []byte) ([]byte, error) { return nil, nil } + // TODO(#388): The lock is necessary because `HandleNetworkData` can be called concurrently + // which has led to `fatal error: concurrent map writes` errors. Once a proper, shared, mempool + // implementation is available, this should no longer be necessary. + n.mempoolLock.Lock() + defer n.mempoolLock.Unlock() + n.nonceSet[rainTreeMsg.Nonce] = struct{}{} n.nonceList = append(n.nonceList, rainTreeMsg.Nonce) - if uint64(len(n.nonceList)) > n.mampoolMaxNonces { + if uint64(len(n.nonceList)) > n.mempoolMaxNonces { // removing the oldest nonce and the oldest key from the slice delete(n.nonceSet, n.nonceList[0]) n.nonceList = n.nonceList[1:] diff --git a/persistence/context.go b/persistence/context.go index 0a8167766..2c35872cf 100644 --- a/persistence/context.go +++ b/persistence/context.go @@ -96,7 +96,7 @@ func (p PostgresContext) Release() error { } func (p PostgresContext) Close() error { - log.Printf("About to close context at height %d.\n", p.Height) + log.Printf("About to close postgres context at height %d.\n", p.Height) return p.conn.Close(context.TODO()) } From 598306804581ec39bdd86f51f44d6f97c3ced4aa Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Thu, 22 Dec 2022 15:29:53 -0500 Subject: [PATCH 03/21] Some documentation improvements --- consensus/hotstuff_leader.go | 5 ++++- consensus/hotstuff_replica.go | 4 +++- consensus/types/errors.go | 6 +++--- persistence/context.go | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/consensus/hotstuff_leader.go b/consensus/hotstuff_leader.go index 3983e09fd..80e66dffc 100644 --- a/consensus/hotstuff_leader.go +++ b/consensus/hotstuff_leader.go @@ -270,8 +270,10 @@ func (handler *HotstuffLeaderMessageHandler) emitTelemetryEvent(m *consensusModu EmitEvent( consensusTelemetry.CONSENSUS_EVENT_METRICS_NAMESPACE, consensusTelemetry.HOTPOKT_MESSAGE_EVENT_METRIC_NAME, - consensusTelemetry.HOTPOKT_MESSAGE_EVENT_METRIC_LABEL_HEIGHT, m.CurrentHeight(), + consensusTelemetry.HOTPOKT_MESSAGE_EVENT_METRIC_LABEL_HEIGHT, + m.CurrentHeight(), typesCons.StepToString[msg.GetStep()], + m.CurrentRound(), consensusTelemetry.HOTPOKT_MESSAGE_EVENT_METRIC_LABEL_VALIDATOR_TYPE_LEADER, ) } @@ -329,6 +331,7 @@ func (m *consensusModule) indexHotstuffMessage(msg *typesCons.HotstuffMessage) e // This is a helper function intended to be called by a leader/validator during a view change // to prepare a new block that is applied to the new underlying context. +// TODO: Split this into atomic `prepareBlock` and `applyBlock` functions func (m *consensusModule) prepareAndApplyBlock(qc *typesCons.QuorumCertificate) (*typesCons.Block, error) { if m.isReplica() { return nil, typesCons.ErrReplicaPrepareBlock diff --git a/consensus/hotstuff_replica.go b/consensus/hotstuff_replica.go index e7c45b083..ec4ca9731 100644 --- a/consensus/hotstuff_replica.go +++ b/consensus/hotstuff_replica.go @@ -179,8 +179,10 @@ func (handler *HotstuffReplicaMessageHandler) emitTelemetryEvent(m *consensusMod EmitEvent( consensusTelemetry.CONSENSUS_EVENT_METRICS_NAMESPACE, consensusTelemetry.HOTPOKT_MESSAGE_EVENT_METRIC_NAME, - consensusTelemetry.HOTPOKT_MESSAGE_EVENT_METRIC_LABEL_HEIGHT, m.CurrentHeight(), + consensusTelemetry.HOTPOKT_MESSAGE_EVENT_METRIC_LABEL_HEIGHT, + m.CurrentHeight(), typesCons.StepToString[msg.GetStep()], + m.CurrentRound(), consensusTelemetry.HOTPOKT_MESSAGE_EVENT_METRIC_LABEL_VALIDATOR_TYPE_REPLICA, ) } diff --git a/consensus/types/errors.go b/consensus/types/errors.go index aff764c76..65046f094 100644 --- a/consensus/types/errors.go +++ b/consensus/types/errors.go @@ -59,7 +59,7 @@ func PacemakerCatchup(height1, step1, round1, height2, step2, round2 uint64) str } func OptimisticVoteCountWaiting(step HotstuffStep, status string) string { - return fmt.Sprintf("Still waiting for more %s messages; %s", StepToString[step], status) + return fmt.Sprintf("Waiting for more %s messages; %s", StepToString[step], status) } func OptimisticVoteCountPassed(step HotstuffStep) string { @@ -71,11 +71,11 @@ func CommittingBlock(height uint64, numTxs int) string { } func ElectedNewLeader(address string, nodeId NodeId, height, round uint64) string { - return fmt.Sprintf("👑 Elected new leader for (%d-%d): %d (%s) 👑", height, round, nodeId, address) + return fmt.Sprintf("🙇🙇🙇 Elected leader for height/round %d/%d: [%d] (%s) 🙇🙇🙇", height, round, nodeId, address) } func ElectedSelfAsNewLeader(address string, nodeId NodeId, height, round uint64) string { - return fmt.Sprintf("👑👑👑👑👑👑 I am the new leader for (%d-%d): %d (%s) 👑👑👑👑👑👑👑👑", height, round, nodeId, address) + return fmt.Sprintf("👑👑👑 I am the leader for height/round %d/%d: [%d] (%s) 👑👑👑", height, round, nodeId, address) } func SendingMessage(msg *HotstuffMessage, nodeId NodeId) string { diff --git a/persistence/context.go b/persistence/context.go index 2c35872cf..07099b24f 100644 --- a/persistence/context.go +++ b/persistence/context.go @@ -84,7 +84,7 @@ func (p PostgresContext) Commit(proposerAddr, quorumCert []byte) error { } func (p PostgresContext) Release() error { - log.Printf("About to release context at height %d.\n", p.Height) + log.Printf("About to release postgres context at height %d.\n", p.Height) ctx := context.TODO() if err := p.getTx().Rollback(ctx); err != nil { return err From 12d8b2b293af1613482aaa9d83e93d6a06e2edab Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Thu, 22 Dec 2022 20:12:31 -0500 Subject: [PATCH 04/21] WIP - refactoring the logs --- consensus/consensus_tests/pacemaker_test.go | 26 +++++++------- consensus/consensus_tests/utils_test.go | 22 ++++++------ consensus/helpers.go | 6 ++-- consensus/hotstuff_handler.go | 4 +-- consensus/hotstuff_leader.go | 15 +++++--- consensus/module.go | 2 +- consensus/pacemaker.go | 38 ++++++++++----------- consensus/types/errors.go | 11 ++++-- 8 files changed, 66 insertions(+), 58 deletions(-) diff --git a/consensus/consensus_tests/pacemaker_test.go b/consensus/consensus_tests/pacemaker_test.go index 17fdd0b93..153313717 100644 --- a/consensus/consensus_tests/pacemaker_test.go +++ b/consensus/consensus_tests/pacemaker_test.go @@ -1,6 +1,7 @@ package consensus_tests import ( + "fmt" "reflect" "runtime" "testing" @@ -17,6 +18,7 @@ import ( ) func TestTinyPacemakerTimeouts(t *testing.T) { + fmt.Println("ATE") clockMock := clock.NewMock() timeReminder(clockMock, 100*time.Millisecond) @@ -35,7 +37,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel) StartAllTestPocketNodes(t, pocketNodes) - // Debug message to start consensus by triggering next view. + // // Debug message to start consensus by triggering next view. for _, pocketNode := range pocketNodes { TriggerNextView(t, pocketNode) } @@ -72,7 +74,8 @@ func TestTinyPacemakerTimeouts(t *testing.T) { } forcePacemakerTimeout(clockMock, paceMakerTimeout) - // // Check that a new round starts at the same height + + // Check that a new round starts at the same height _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { @@ -86,7 +89,6 @@ func TestTinyPacemakerTimeouts(t *testing.T) { } forcePacemakerTimeout(clockMock, paceMakerTimeout) - // Check that a new round starts at the same height. newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) require.NoError(t, err) @@ -111,15 +113,15 @@ func TestTinyPacemakerTimeouts(t *testing.T) { // Confirm we are at the next step _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 500) require.NoError(t, err) - for pocketId, pocketNode := range pocketNodes { - assertNodeConsensusView(t, pocketId, - typesCons.ConsensusNodeState{ - Height: 1, - Step: uint8(consensus.Prepare), - Round: 3, - }, - GetConsensusNodeState(pocketNode)) - } + // for pocketId, pocketNode := range pocketNodes { + // assertNodeConsensusView(t, pocketId, + // typesCons.ConsensusNodeState{ + // Height: 1, + // Step: uint8(consensus.Prepare), + // Round: 3, + // }, + // GetConsensusNodeState(pocketNode)) + // } } func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { diff --git a/consensus/consensus_tests/utils_test.go b/consensus/consensus_tests/utils_test.go index 9cf3c2de1..c465488e5 100644 --- a/consensus/consensus_tests/utils_test.go +++ b/consensus/consensus_tests/utils_test.go @@ -265,14 +265,14 @@ loop: continue } - message := testEvent.Content - if message == nil || !includeFilter(message) { - unused = append(unused, &testEvent) - continue - } + // message := testEvent.Content + // if message == nil || !includeFilter(message) { + // unused = append(unused, &testEvent) + // continue + // } - messages = append(messages, message) - numMessages-- + // messages = append(messages, message) + // numMessages-- // The if structure below "breaks early" when we get enough messages. However, it does not capture // the case where we could be receiving more messages than expected. To make sure the latter doesn't @@ -292,10 +292,10 @@ loop: } } } - cancel() - for _, u := range unused { - testChannel <- *u - } + // cancel() + // for _, u := range unused { + // testChannel <- *u + // } return } diff --git a/consensus/helpers.go b/consensus/helpers.go index 1d26f9159..4816d1be3 100644 --- a/consensus/helpers.go +++ b/consensus/helpers.go @@ -169,7 +169,6 @@ func (m *consensusModule) broadcastToValidators(msg *typesCons.HotstuffMessage) } for _, val := range m.validatorMap { - // fmt.Println("OLSH", addr) if err := m.GetBus().GetP2PModule().Send(cryptoPocket.AddressFromString(val.GetAddress()), anyConsensusMessage); err != nil { m.nodeLogError(typesCons.ErrBroadcastMessage.Error(), err) } @@ -178,7 +177,7 @@ func (m *consensusModule) broadcastToValidators(msg *typesCons.HotstuffMessage) /*** Persistence Helpers ***/ -// TECHDEBT: Integrate this with the `persistence` module or a real mempool. +// TECHDEBT(#388): Integrate this with the `persistence` module or a real mempool. func (m *consensusModule) clearMessagesPool() { for _, step := range HotstuffSteps { m.messagePool[step] = make([]*typesCons.HotstuffMessage, 0) @@ -211,7 +210,6 @@ func (m *consensusModule) electNextLeader(message *typesCons.HotstuffMessage) er m.clearLeader() return err } - m.leaderId = &leaderId if m.isLeader() { @@ -234,7 +232,7 @@ func (m *consensusModule) nodeLog(s string) { // TODO(#164): Remove this once we have a proper logging system. func (m *consensusModule) nodeLogError(s string, err error) { - log.Printf("[ERROR][%s][%d] %s: %v\n", m.logPrefix, m.nodeId, s, err) + log.Printf("🐞[ERROR][%s][%d] %s: %v\n", m.logPrefix, m.nodeId, s, err) } func (m *consensusModule) setLogPrefix(logPrefix string) { diff --git a/consensus/hotstuff_handler.go b/consensus/hotstuff_handler.go index 9a2e74095..ef245b50a 100644 --- a/consensus/hotstuff_handler.go +++ b/consensus/hotstuff_handler.go @@ -14,14 +14,14 @@ type HotstuffMessageHandler interface { } func (m *consensusModule) handleHotstuffMessage(msg *typesCons.HotstuffMessage) error { - m.nodeLog(typesCons.DebugHandlingHotstuffMessage(msg)) - step := msg.GetStep() + m.nodeLog(typesCons.DebugReceivedHandlingHotstuffMessage(msg)) // Pacemaker - Liveness & safety checks if shouldHandle, err := m.paceMaker.ShouldHandleMessage(msg); !shouldHandle { return err } + m.nodeLog(typesCons.DebugHandlingHotstuffMessage(msg)) // Elect a leader for the current round if needed if m.shouldElectNextLeader() { diff --git a/consensus/hotstuff_leader.go b/consensus/hotstuff_leader.go index 80e66dffc..1593e3822 100644 --- a/consensus/hotstuff_leader.go +++ b/consensus/hotstuff_leader.go @@ -279,20 +279,25 @@ func (handler *HotstuffLeaderMessageHandler) emitTelemetryEvent(m *consensusModu } func (m *consensusModule) validateMessageSignature(msg *typesCons.HotstuffMessage) error { + partialSig := msg.GetPartialSignature() + if msg.GetStep() == NewRound { - m.nodeLog(typesCons.ErrUnnecessaryPartialSigForNewRound.Error()) + if partialSig != nil { + m.nodeLog(typesCons.ErrUnnecessaryPartialSigForNewRound.Error()) + } return nil } if msg.GetType() == Propose { - m.nodeLog(typesCons.ErrUnnecessaryPartialSigForLeaderProposal.Error()) + if partialSig != nil { + m.nodeLog(typesCons.ErrUnnecessaryPartialSigForLeaderProposal.Error()) + } return nil } - if msg.GetPartialSignature() == nil { + if partialSig == nil { return typesCons.ErrNilPartialSig } - partialSig := msg.GetPartialSignature() if partialSig.Signature == nil || len(partialSig.GetAddress()) == 0 { return typesCons.ErrNilPartialSigOrSourceNotSpecified @@ -331,7 +336,7 @@ func (m *consensusModule) indexHotstuffMessage(msg *typesCons.HotstuffMessage) e // This is a helper function intended to be called by a leader/validator during a view change // to prepare a new block that is applied to the new underlying context. -// TODO: Split this into atomic `prepareBlock` and `applyBlock` functions +// TODO: Split this into atomic & functional `prepareBlock` and `applyBlock` methods func (m *consensusModule) prepareAndApplyBlock(qc *typesCons.QuorumCertificate) (*typesCons.Block, error) { if m.isReplica() { return nil, typesCons.ErrReplicaPrepareBlock diff --git a/consensus/module.go b/consensus/module.go index 51242f5c1..faabeab4f 100644 --- a/consensus/module.go +++ b/consensus/module.go @@ -325,7 +325,7 @@ func (m *consensusModule) loadPersistedState() error { m.height = uint64(latestHeight) + 1 // +1 because the height of the consensus module is where it is actively participating in consensus - m.nodeLog(fmt.Sprintf("Starting node at height %d", latestHeight)) + m.nodeLog(fmt.Sprintf("Starting consensus module at height %d", latestHeight)) return nil } diff --git a/consensus/pacemaker.go b/consensus/pacemaker.go index f0752b83a..0611775ec 100644 --- a/consensus/pacemaker.go +++ b/consensus/pacemaker.go @@ -117,57 +117,55 @@ func (m *paceMaker) SetConsensusModule(c *consensusModule) { m.consensusMod = c } -func (p *paceMaker) ShouldHandleMessage(m *typesCons.HotstuffMessage) (bool, error) { +func (p *paceMaker) ShouldHandleMessage(msg *typesCons.HotstuffMessage) (bool, error) { currentHeight := p.consensusMod.height currentRound := p.consensusMod.round + currentStep := p.consensusMod.step // Consensus message is from the past - if m.Height < currentHeight { - // TODO: Noisy log - make it a DEBUG - // p.consensusMod.nodeLog(typesCons.ErrPacemakerUnexpectedMessageHeight(typesCons.ErrOlderMessage, currentHeight, m.Height).Error()) + if msg.Height < currentHeight { + p.consensusMod.nodeLog(fmt.Sprintf("[WARN][DISCARDING] Node at height %d received message at a %d", currentHeight, msg.Height)) return false, nil } - // Current node is out of sync // TODO: Need to restart state sync or be in state sync mode right now - if m.Height > currentHeight { - // TODO: Noisy log - make it a DEBUG - // p.consensusMod.nodeLog(typesCons.ErrPacemakerUnexpectedMessageHeight(typesCons.ErrFutureMessage, currentHeight, m.Height).Error()) + // Current node is out of sync + if msg.Height > currentHeight { + p.consensusMod.nodeLog(fmt.Sprintf("[WARN][DISCARDING] Node at height %d received message at a %d", currentHeight, msg.Height)) return false, nil } - // Do not handle messages if it is a self proposal // TODO(olshansky): This code branch is a result of the optimization in the leader // handlers. Since the leader also acts as a replica but doesn't use the replica's // handlers given the current implementation, it is safe to drop proposal that the leader made to itself. - if p.consensusMod.isLeader() && m.Type == Propose && m.Step != NewRound { + // Do not handle messages if it is a self proposal + if p.consensusMod.isLeader() && msg.Type == Propose && msg.Step != NewRound { // TODO: Noisy log - make it a DEBUG // p.consensusMod.nodeLog(typesCons.ErrSelfProposal.Error()) return false, nil } // Message is from the past - if m.Round < currentRound || (m.Round == currentRound && m.Step < p.consensusMod.step) { - // TODO: Noisy log - make it a DEBUG - // p.consensusMod.nodeLog(typesCons.ErrPacemakerUnexpectedMessageStepRound(typesCons.ErrOlderStepRound, p.consensusMod.step, currentRound, m).Error()) + if msg.Round < currentRound || (msg.Round == currentRound && msg.Step < currentStep) { + p.consensusMod.nodeLog(fmt.Sprintf("[WARN][DISCARDING] Node at (step, round) (%d, %d, %d) received message at (%d, %d, %d)", currentHeight, currentRound, currentHeight, currentStep, msg.Round, msg.Step)) return false, nil } // Everything checks out! - if m.Height == currentHeight && m.Step == p.consensusMod.step && m.Round == currentRound { + if msg.Height == currentHeight && msg.Step == currentStep && msg.Round == currentRound { return true, nil } // Pacemaker catch up! Node is synched to the right height, but on a previous step/round so we just jump to the latest state. - if m.Round > currentRound || (m.Round == currentRound && m.Step > p.consensusMod.step) { - p.consensusMod.nodeLog(typesCons.PacemakerCatchup(currentHeight, uint64(p.consensusMod.step), currentRound, m.Height, uint64(m.Step), m.Round)) - p.consensusMod.step = m.Step - p.consensusMod.round = m.Round + if msg.Round > currentRound || (msg.Round == currentRound && msg.Step > currentStep) { + p.consensusMod.nodeLog(typesCons.PacemakerCatchup(currentHeight, uint64(currentStep), currentRound, msg.Height, uint64(msg.Step), msg.Round)) + p.consensusMod.step = msg.Step + p.consensusMod.round = msg.Round // TODO(olshansky): Add tests for this. When we catch up to a later step, the leader is still the same. // However, when we catch up to a later round, the leader at the same height will be different. - if currentRound != m.Round || p.consensusMod.leaderId == nil { - p.consensusMod.electNextLeader(m) + if currentRound != msg.Round || p.consensusMod.leaderId == nil { + p.consensusMod.electNextLeader(msg) } return true, nil diff --git a/consensus/types/errors.go b/consensus/types/errors.go index 65046f094..e08d62a49 100644 --- a/consensus/types/errors.go +++ b/consensus/types/errors.go @@ -20,7 +20,7 @@ const ( ProposalBlockExtends = "the ProposalQC block is the same as the LockedQC block" // WARN - NilUtilityContextWarning = "[WARN] Utility context not nil when preparing a new block? Releasing for now but should not happen" + NilUtilityContextWarning = "[WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it" InvalidPartialSigInQCWarning = "[WARN] QC contains an invalid partial signature" // DEBUG @@ -114,9 +114,14 @@ func DebugNodeState(state ConsensusNodeState) string { return fmt.Sprintf("[DEBUG] Node %d is at (Height, Step, Round): (%d, %d, %d)\n", state.NodeId, state.Height, state.Step, state.Round) } +// TODO(olshansky): Add source and destination NodeId of message here +func DebugReceivedHandlingHotstuffMessage(msg *HotstuffMessage) string { + return fmt.Sprintf("[DEBUG] Received hotstuff msg at (Height, Step, Round): (%d, %d, %d)\n", msg.Height, msg.GetStep(), msg.Round) +} + +// TODO(olshansky): Add source and destination NodeId of message here func DebugHandlingHotstuffMessage(msg *HotstuffMessage) string { - // TODO(olshansky): Add source and destination NodeId of message here - return fmt.Sprintf("[DEBUG] Handling message w/ Height: %d; Type: %s; Round: %d.", msg.Height, StepToString[msg.GetStep()], msg.Round) + return fmt.Sprintf("[DEBUG] Handling hotstuff msg at (Height, Step, Round): (%d, %d, %d)\n", msg.Height, msg.GetStep(), msg.Round) } // Errors From 3b74cc43a86241041d23c319e940b56e298961b9 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Fri, 23 Dec 2022 16:17:49 -0500 Subject: [PATCH 05/21] Improved consensus test logging and renamed top level directory --- Makefile | 17 +++++--- consensus/doc/CHANGELOG.md | 2 +- .../hotstuff_test.go | 22 +++++----- .../pacemaker_test.go | 35 ++++++++-------- .../utils_test.go | 42 +++++++++---------- 5 files changed, 62 insertions(+), 56 deletions(-) rename consensus/{consensus_tests => e2e_tests}/hotstuff_test.go (93%) rename consensus/{consensus_tests => e2e_tests}/pacemaker_test.go (90%) rename consensus/{consensus_tests => e2e_tests}/utils_test.go (95%) diff --git a/Makefile b/Makefile index a2b2638ec..9f1087987 100644 --- a/Makefile +++ b/Makefile @@ -305,20 +305,25 @@ test_shared: ## Run all go unit tests in the shared module test_consensus: ## Run all go unit tests in the consensus module go test ${VERBOSE_TEST} -count=1 ./consensus/... +# These tests are isolated to a single package which enables logs to be streamed in realtime. More details here: https://stackoverflow.com/a/74903989/768439 +.PHONY: test_consensus_e2e +test_consensus_e2e: ## Run all go t2e unit tests in the consensus module w/ log streaming + go test ${VERBOSE_TEST} -count=1 ./consensus/e2e_tests/... + .PHONY: test_consensus_concurrent_tests test_consensus_concurrent_tests: ## Run unit tests in the consensus module that could be prone to race conditions (#192) - for i in $$(seq 1 100); do go test -timeout 2s -count=1 -run ^TestTinyPacemakerTimeouts$ ./consensus/consensus_tests; done; - for i in $$(seq 1 100); do go test -timeout 2s -count=1 -run ^TestHotstuff4Nodes1BlockHappyPath$ ./consensus/consensus_tests; done; - for i in $$(seq 1 100); do go test -timeout 2s -count=1 -race -run ^TestTinyPacemakerTimeouts$ ./consensus/consensus_tests; done; - for i in $$(seq 1 100); do go test -timeout 2s -count=1 -race -run ^TestHotstuff4Nodes1BlockHappyPath$ ./consensus/consensus_tests; done; + for i in $$(seq 1 100); do go test -timeout 2s -count=1 -run ^TestTinyPacemakerTimeouts$ ./consensus/e2e_tests; done; + for i in $$(seq 1 100); do go test -timeout 2s -count=1 -run ^TestHotstuff4Nodes1BlockHappyPath$ ./consensus/e2e_tests; done; + for i in $$(seq 1 100); do go test -timeout 2s -count=1 -race -run ^TestTinyPacemakerTimeouts$ ./consensus/e2e_tests; done; + for i in $$(seq 1 100); do go test -timeout 2s -count=1 -race -run ^TestHotstuff4Nodes1BlockHappyPath$ ./consensus/e2e_tests; done; .PHONY: test_hotstuff test_hotstuff: ## Run all go unit tests related to hotstuff consensus - go test ${VERBOSE_TEST} ./consensus/consensus_tests -run Hotstuff -failOnExtraMessages=${EXTRA_MSG_FAIL} + go test ${VERBOSE_TEST} ./consensus/e2e_tests -run Hotstuff -failOnExtraMessages=${EXTRA_MSG_FAIL} .PHONY: test_pacemaker test_pacemaker: ## Run all go unit tests related to the hotstuff pacemaker - go test ${VERBOSE_TEST} ./consensus/consensus_tests -run Pacemaker -failOnExtraMessages=${EXTRA_MSG_FAIL} + go test ${VERBOSE_TEST} ./consensus/e2e_tests -run Pacemaker -failOnExtraMessages=${EXTRA_MSG_FAIL} .PHONY: test_vrf test_vrf: ## Run all go unit tests in the VRF library diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index 7c4267f40..25925d73f 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -89,7 +89,7 @@ Consensus cleanup Consensus testing -- Improved mock module initialization in `consensus/consensus_tests/utils_test.go` +- Improved mock module initialization in `consensus/e2e_tests/utils_test.go` General diff --git a/consensus/consensus_tests/hotstuff_test.go b/consensus/e2e_tests/hotstuff_test.go similarity index 93% rename from consensus/consensus_tests/hotstuff_test.go rename to consensus/e2e_tests/hotstuff_test.go index 12a8861b6..24e610c3a 100644 --- a/consensus/consensus_tests/hotstuff_test.go +++ b/consensus/e2e_tests/hotstuff_test.go @@ -1,4 +1,4 @@ -package consensus_tests +package e2e_tests import ( "fmt" @@ -17,7 +17,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { // Test configs runtimeMgrs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) - go timeReminder(clockMock, 100*time.Millisecond) + go timeReminder(t, clockMock, 100*time.Millisecond) // Create & start test pocket nodes testChannel := make(modules.EventsChannel, 100) @@ -29,7 +29,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { TriggerNextView(t, pocketNode) } - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) // NewRound newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 1000) @@ -54,7 +54,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { leaderId := typesCons.NodeId(2) leader := pocketNodes[leaderId] - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) // Prepare prepareProposal, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 1000) @@ -74,7 +74,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { P2PBroadcast(t, pocketNodes, message) } - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) // Precommit prepareVotes, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Vote, numValidators, 1000) @@ -83,7 +83,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { P2PSend(t, leader, vote) } - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) preCommitProposal, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.PreCommit, consensus.Propose, 1, 1000) require.NoError(t, err) @@ -102,7 +102,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { P2PBroadcast(t, pocketNodes, message) } - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) // Commit preCommitVotes, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.PreCommit, consensus.Vote, numValidators, 1000) @@ -111,7 +111,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { P2PSend(t, leader, vote) } - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) commitProposal, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Commit, consensus.Propose, 1, 1000) require.NoError(t, err) @@ -130,7 +130,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { P2PBroadcast(t, pocketNodes, message) } - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) // Decide commitVotes, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Commit, consensus.Vote, numValidators, 1000) @@ -139,7 +139,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { P2PSend(t, leader, vote) } - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) decideProposal, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Decide, consensus.Propose, 1, 1000) require.NoError(t, err) @@ -170,7 +170,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { P2PBroadcast(t, pocketNodes, message) } - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) // Block has been committed and new round has begun _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 1000) diff --git a/consensus/consensus_tests/pacemaker_test.go b/consensus/e2e_tests/pacemaker_test.go similarity index 90% rename from consensus/consensus_tests/pacemaker_test.go rename to consensus/e2e_tests/pacemaker_test.go index 153313717..ac9a7563d 100644 --- a/consensus/consensus_tests/pacemaker_test.go +++ b/consensus/e2e_tests/pacemaker_test.go @@ -1,7 +1,6 @@ -package consensus_tests +package e2e_tests import ( - "fmt" "reflect" "runtime" "testing" @@ -9,7 +8,6 @@ import ( timePkg "time" "github.com/benbjohnson/clock" - "github.com/pokt-network/pocket/consensus" typesCons "github.com/pokt-network/pocket/consensus/types" "github.com/pokt-network/pocket/shared/modules" @@ -18,9 +16,10 @@ import ( ) func TestTinyPacemakerTimeouts(t *testing.T) { - fmt.Println("ATE") + t.Parallel() + clockMock := clock.NewMock() - timeReminder(clockMock, 100*time.Millisecond) + timeReminder(t, clockMock, 100*time.Millisecond) // Test configs paceMakerTimeoutMsec := uint64(50) // Set a very small pacemaker timeout @@ -43,7 +42,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { } // advance time by an amount shorter than the timeout - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) // paceMakerTimeout _, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) @@ -58,7 +57,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { GetConsensusNodeState(pocketNode)) } - forcePacemakerTimeout(clockMock, paceMakerTimeout) + forcePacemakerTimeout(t, clockMock, paceMakerTimeout) // Check that a new round starts at the same height. _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) @@ -73,7 +72,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { GetConsensusNodeState(pocketNode)) } - forcePacemakerTimeout(clockMock, paceMakerTimeout) + forcePacemakerTimeout(t, clockMock, paceMakerTimeout) // Check that a new round starts at the same height _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) @@ -88,7 +87,8 @@ func TestTinyPacemakerTimeouts(t *testing.T) { GetConsensusNodeState(pocketNode)) } - forcePacemakerTimeout(clockMock, paceMakerTimeout) + forcePacemakerTimeout(t, clockMock, paceMakerTimeout) + // Check that a new round starts at the same height. newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) require.NoError(t, err) @@ -108,11 +108,12 @@ func TestTinyPacemakerTimeouts(t *testing.T) { } // advance time by an amount shorter than the timeout - advanceTime(clockMock, 10*time.Millisecond) + advanceTime(t, clockMock, 10*time.Millisecond) + time.Sleep(10 * time.Second) // Confirm we are at the next step - _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 500) - require.NoError(t, err) + // _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 500) + // require.NoError(t, err) // for pocketId, pocketNode := range pocketNodes { // assertNodeConsensusView(t, pocketId, // typesCons.ConsensusNodeState{ @@ -128,7 +129,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { clockMock := clock.NewMock() runtimeConfigs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) - timeReminder(clockMock, 100*time.Millisecond) + timeReminder(t, clockMock, 100*time.Millisecond) // Create & start test pocket nodes testChannel := make(modules.EventsChannel, 100) @@ -201,7 +202,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Vote, numValidators-1, 2000) require.NoError(t, err) - forcePacemakerTimeout(clockMock, 600*time.Millisecond) + forcePacemakerTimeout(t, clockMock, 600*time.Millisecond) // Check that the leader is in the latest round. for nodeId, pocketNode := range pocketNodes { @@ -251,12 +252,12 @@ func TestPacemakerExponentialTimeouts(t *testing.T) { } */ -func forcePacemakerTimeout(clockMock *clock.Mock, paceMakerTimeout timePkg.Duration) { +func forcePacemakerTimeout(t *testing.T, clockMock *clock.Mock, paceMakerTimeout timePkg.Duration) { go func() { // Cause the pacemaker to timeout - sleep(clockMock, paceMakerTimeout) + sleep(t, clockMock, paceMakerTimeout) }() runtime.Gosched() // advance time by an amount longer than the timeout - advanceTime(clockMock, paceMakerTimeout+10*time.Millisecond) + advanceTime(t, clockMock, paceMakerTimeout+10*time.Millisecond) } diff --git a/consensus/consensus_tests/utils_test.go b/consensus/e2e_tests/utils_test.go similarity index 95% rename from consensus/consensus_tests/utils_test.go rename to consensus/e2e_tests/utils_test.go index c465488e5..584baaafb 100644 --- a/consensus/consensus_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -1,4 +1,4 @@ -package consensus_tests +package e2e_tests import ( "context" @@ -265,14 +265,14 @@ loop: continue } - // message := testEvent.Content - // if message == nil || !includeFilter(message) { - // unused = append(unused, &testEvent) - // continue - // } + message := testEvent.Content + if message == nil || !includeFilter(message) { + unused = append(unused, &testEvent) + continue + } - // messages = append(messages, message) - // numMessages-- + messages = append(messages, message) + numMessages-- // The if structure below "breaks early" when we get enough messages. However, it does not capture // the case where we could be receiving more messages than expected. To make sure the latter doesn't @@ -292,10 +292,10 @@ loop: } } } - // cancel() - // for _, u := range unused { - // testChannel <- *u - // } + cancel() + for _, u := range unused { + testChannel <- *u + } return } @@ -437,32 +437,32 @@ func baseLoggerMock(t *testing.T, _ modules.EventsChannel) *modulesMock.MockLogg return loggerMock } -func logTime(clock *clock.Mock) { - log.Printf("[⌚ CLOCK ⌚] the time is: %v ms from UNIX Epoch [%v]", clock.Now().UTC().UnixMilli(), clock.Now().UTC()) +func logTime(t *testing.T, clock *clock.Mock) { + t.Logf("[⌚ CLOCK ⌚] the time is: %v ms from UNIX Epoch [%v]", clock.Now().UTC().UnixMilli(), clock.Now().UTC()) } // advanceTime moves the time forward on the mock clock and logs what just happened. -func advanceTime(clock *clock.Mock, duration time.Duration) { +func advanceTime(t *testing.T, clock *clock.Mock, duration time.Duration) { clock.Add(duration) - log.Printf("[⌚ CLOCK ⏩] advanced by %v", duration) - logTime(clock) + t.Logf("[⌚ CLOCK ⏩] advanced by %v", duration) + logTime(t, clock) } // sleep pauses the goroutine for the given duration on the mock clock and logs what just happened. // // Note: time has to be moved forward in a separate goroutine, see `advanceTime`. -func sleep(clock *clock.Mock, duration time.Duration) { - log.Printf("[⌚ CLOCK 💤] sleeping for %v", duration) +func sleep(t *testing.T, clock *clock.Mock, duration time.Duration) { + t.Logf("[⌚ CLOCK 💤] sleeping for %v", duration) clock.Sleep(duration) } // timeReminder simply prints, at a given interval and in a separate goroutine, the current mocked time to help with events. -func timeReminder(clock *clock.Mock, frequency time.Duration) { +func timeReminder(t *testing.T, clock *clock.Mock, frequency time.Duration) { go func() { tick := time.NewTicker(frequency) for { <-tick.C - logTime(clock) + logTime(t, clock) } }() } From cb2484ea8b2345f529cec15145012171fa06d8dc Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Sat, 24 Dec 2022 15:41:31 -0500 Subject: [PATCH 06/21] Got the consensus test timeouts working again but the pacemaker test still fails --- consensus/e2e_tests/pacemaker_test.go | 26 ++++--- consensus/e2e_tests/utils_test.go | 82 ++++++++++++----------- consensus/hotstuff_leader.go | 8 +-- consensus/leader_election/vrf/vrf_test.go | 6 +- consensus/pacemaker.go | 2 +- consensus/types/errors.go | 48 ++++++------- 6 files changed, 88 insertions(+), 84 deletions(-) diff --git a/consensus/e2e_tests/pacemaker_test.go b/consensus/e2e_tests/pacemaker_test.go index ac9a7563d..045dfe075 100644 --- a/consensus/e2e_tests/pacemaker_test.go +++ b/consensus/e2e_tests/pacemaker_test.go @@ -16,8 +16,6 @@ import ( ) func TestTinyPacemakerTimeouts(t *testing.T) { - t.Parallel() - clockMock := clock.NewMock() timeReminder(t, clockMock, 100*time.Millisecond) @@ -104,25 +102,25 @@ func TestTinyPacemakerTimeouts(t *testing.T) { // Continue to the next step at the current round for _, message := range newRoundMessages { + // msg, _ := codec.GetCodec().FromAny(message) P2PBroadcast(t, pocketNodes, message) } // advance time by an amount shorter than the timeout advanceTime(t, clockMock, 10*time.Millisecond) - time.Sleep(10 * time.Second) // Confirm we are at the next step - // _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 500) - // require.NoError(t, err) - // for pocketId, pocketNode := range pocketNodes { - // assertNodeConsensusView(t, pocketId, - // typesCons.ConsensusNodeState{ - // Height: 1, - // Step: uint8(consensus.Prepare), - // Round: 3, - // }, - // GetConsensusNodeState(pocketNode)) - // } + _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 500) + require.NoError(t, err) + for pocketId, pocketNode := range pocketNodes { + assertNodeConsensusView(t, pocketId, + typesCons.ConsensusNodeState{ + Height: 1, + Step: uint8(consensus.Prepare), + Round: 3, + }, + GetConsensusNodeState(pocketNode)) + } } func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index 584baaafb..0bde7d022 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -219,81 +219,87 @@ func P2PSend(_ *testing.T, node *shared.Node, any *anypb.Any) { func WaitForNetworkConsensusMessages( t *testing.T, - clock clock.Clock, + clock *clock.Mock, testChannel modules.EventsChannel, step typesCons.HotstuffStep, - hotstuffMsgType typesCons.HotstuffMessageType, - numMessages int, + msgType typesCons.HotstuffMessageType, + numExpectedMsgs int, millis time.Duration, ) (messages []*anypb.Any, err error) { - - includeFilter := func(m *anypb.Any) bool { - msg, err := codec.GetCodec().FromAny(m) + includeFilter := func(anyMsg *anypb.Any) bool { + msg, err := codec.GetCodec().FromAny(anyMsg) require.NoError(t, err) hotstuffMessage, ok := msg.(*typesCons.HotstuffMessage) require.True(t, ok) - return hotstuffMessage.Type == hotstuffMsgType && hotstuffMessage.Step == step + return hotstuffMessage.Type == msgType && hotstuffMessage.Step == step } - errorMessage := fmt.Sprintf("HotStuff step: %s, type: %s", typesCons.HotstuffStep_name[int32(step)], typesCons.HotstuffMessageType_name[int32(hotstuffMsgType)]) - return waitForNetworkConsensusMessagesInternal(t, clock, testChannel, consensus.HotstuffMessageContentType, numMessages, millis, includeFilter, errorMessage) + errorMessage := fmt.Sprintf("HotStuff step: %s, type: %s", typesCons.HotstuffStep_name[int32(step)], typesCons.HotstuffMessageType_name[int32(msgType)]) + return waitForNetworkConsensusMessagesInternal(t, clock, testChannel, consensus.HotstuffMessageContentType, numExpectedMsgs, millis, includeFilter, errorMessage) } // IMPROVE(olshansky): Translate this to use generics. func waitForNetworkConsensusMessagesInternal( - _ *testing.T, - clock clock.Clock, + t *testing.T, + clock *clock.Mock, testChannel modules.EventsChannel, - messageContentType string, - numMessages int, + eventContentType string, + numExpectedMsgs int, millis time.Duration, includeFilter func(m *anypb.Any) bool, - errorMessage string, -) (messages []*anypb.Any, err error) { - messages = make([]*anypb.Any, 0) - ctx, cancel := clock.WithTimeout(context.Background(), time.Millisecond*millis) - unused := make([]*messaging.PocketEnvelope, 0) // TODO: Move this into a pool rather than resending back to the eventbus. + errorMsg string, +) (expectedMsgs []*anypb.Any, err error) { + expectedMsgs = make([]*anypb.Any, 0) // Aggregate and return the messages we're waiting for + unusedEvents := make([]*messaging.PocketEnvelope, 0) // "Recycle" events back into the events channel if we're not using them + // Limit the amount of time we're waiting for the messages to be published on the events channel + ctx, cancel := clock.WithTimeout(context.TODO(), time.Millisecond*millis) + defer cancel() + + // Since the tests use a mock clock, we need to manually advance the clock to trigger the timeout + ticker := clock.Ticker(time.Millisecond) + go func() { + for { + <-ticker.C + clock.Add(time.Millisecond) + } + }() loop: for { select { case testEvent := <-testChannel: - if testEvent.GetContentType() != messageContentType { - unused = append(unused, &testEvent) + if testEvent.GetContentType() != eventContentType { + unusedEvents = append(unusedEvents, &testEvent) continue } message := testEvent.Content if message == nil || !includeFilter(message) { - unused = append(unused, &testEvent) + unusedEvents = append(unusedEvents, &testEvent) continue } - messages = append(messages, message) - numMessages-- - - // The if structure below "breaks early" when we get enough messages. However, it does not capture - // the case where we could be receiving more messages than expected. To make sure the latter doesn't - // happen, the `failOnExtraMessages` flag must be set to true. - if !failOnExtraMessages && numMessages == 0 { + expectedMsgs = append(expectedMsgs, message) + numExpectedMsgs-- + // Break if: + // 1. We are not expecting any more messages + // 2. We do not want to fail on extra messages + if numExpectedMsgs == 0 && !failOnExtraMessages { break loop } case <-ctx.Done(): - if numMessages == 0 { + if numExpectedMsgs == 0 { break loop - } else if numMessages > 0 { - cancel() - return nil, fmt.Errorf("Missing %s messages; missing: %d, received: %d; (%s)", messageContentType, numMessages, len(messages), errorMessage) + } else if numExpectedMsgs > 0 { + t.Fatalf("Missing %s messages; missing: %d, received: %d; (%s)", eventContentType, numExpectedMsgs, len(expectedMsgs), errorMsg) } else { - cancel() - return nil, fmt.Errorf("Too many %s messages received; expected: %d, received: %d; (%s)", messageContentType, numMessages+len(messages), len(messages), errorMessage) + t.Fatalf("Too many %s messages received; expected: %d, received: %d; (%s)", eventContentType, numExpectedMsgs+len(expectedMsgs), len(expectedMsgs), errorMsg) } } } - cancel() - for _, u := range unused { + for _, u := range unusedEvents { testChannel <- *u } return @@ -310,7 +316,7 @@ func basePersistenceMock(t *testing.T, _ modules.EventsChannel) *modulesMock.Moc persistenceMock.EXPECT().Start().Return(nil).AnyTimes() persistenceMock.EXPECT().SetBus(gomock.Any()).Return().AnyTimes() - persistenceMock.EXPECT().NewReadContext(int64(-1)).Return(persistenceReadContextMock, nil).AnyTimes() + persistenceMock.EXPECT().NewReadContext(gomock.Any()).Return(persistenceReadContextMock, nil).AnyTimes() persistenceMock.EXPECT().ReleaseWriteContext().Return(nil).AnyTimes() // The persistence context should usually be accessed via the utility module within the context @@ -424,7 +430,7 @@ func baseTelemetryTimeSeriesAgentMock(t *testing.T) *modulesMock.MockTimeSeriesA func baseTelemetryEventMetricsAgentMock(t *testing.T) *modulesMock.MockEventMetricsAgent { ctrl := gomock.NewController(t) eventMetricsAgentMock := modulesMock.NewMockEventMetricsAgent(ctrl) - eventMetricsAgentMock.EXPECT().EmitEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + eventMetricsAgentMock.EXPECT().EmitEvent(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() return eventMetricsAgentMock } diff --git a/consensus/hotstuff_leader.go b/consensus/hotstuff_leader.go index 1593e3822..f324324af 100644 --- a/consensus/hotstuff_leader.go +++ b/consensus/hotstuff_leader.go @@ -40,7 +40,7 @@ func (handler *HotstuffLeaderMessageHandler) HandleNewRoundMessage(m *consensusM m.nodeLog(typesCons.OptimisticVoteCountWaiting(NewRound, err.Error())) return } - m.nodeLog(typesCons.OptimisticVoteCountPassed(NewRound)) + m.nodeLog(typesCons.OptimisticVoteCountPassed(m.height, NewRound, m.round)) // Clear the previous utility context, if it exists, and create a new one if err := m.refreshUtilityContext(); err != nil { @@ -108,7 +108,7 @@ func (handler *HotstuffLeaderMessageHandler) HandlePrepareMessage(m *consensusMo m.nodeLog(typesCons.OptimisticVoteCountWaiting(Prepare, err.Error())) return } - m.nodeLog(typesCons.OptimisticVoteCountPassed(Prepare)) + m.nodeLog(typesCons.OptimisticVoteCountPassed(m.height, Prepare, m.round)) prepareQC, err := m.getQuorumCertificate(m.height, Prepare, m.round) if err != nil { @@ -152,7 +152,7 @@ func (handler *HotstuffLeaderMessageHandler) HandlePrecommitMessage(m *consensus m.nodeLog(typesCons.OptimisticVoteCountWaiting(PreCommit, err.Error())) return } - m.nodeLog(typesCons.OptimisticVoteCountPassed(PreCommit)) + m.nodeLog(typesCons.OptimisticVoteCountPassed(m.height, PreCommit, m.round)) preCommitQC, err := m.getQuorumCertificate(m.height, PreCommit, m.round) if err != nil { @@ -196,7 +196,7 @@ func (handler *HotstuffLeaderMessageHandler) HandleCommitMessage(m *consensusMod m.nodeLog(typesCons.OptimisticVoteCountWaiting(Commit, err.Error())) return } - m.nodeLog(typesCons.OptimisticVoteCountPassed(Commit)) + m.nodeLog(typesCons.OptimisticVoteCountPassed(m.height, Commit, m.round)) commitQC, err := m.getQuorumCertificate(m.height, Commit, m.round) if err != nil { diff --git a/consensus/leader_election/vrf/vrf_test.go b/consensus/leader_election/vrf/vrf_test.go index 45a6b51d1..05a340982 100644 --- a/consensus/leader_election/vrf/vrf_test.go +++ b/consensus/leader_election/vrf/vrf_test.go @@ -17,7 +17,7 @@ func TestVRFKeygenWithoutSeed(t *testing.T) { } func TestVRFKeygenWithSeed(t *testing.T) { - seed := "Olshansky wonders if anyone is ever going to read this code" + seed := "👊 if you are reading this and bonus points if you have ideas for how to improve the tests" require.GreaterOrEqual(t, len(seed), crypto.SeedSize/2) privKey, err := crypto.NewPrivateKeyFromSeed([]byte(seed)) @@ -31,8 +31,8 @@ func TestVRFKeygenWithSeed(t *testing.T) { sk, vk, err := GenerateVRFKeys(reader) require.Nil(t, err) - require.Equal(t, "4f6c7368616e736b7920776f6e64657200000000000000000000000000000000c8491df826eccf7557467c74f7a93bb324a15efd2359dc27e3eba940127ff8a2", hex.EncodeToString(sk.Bytes())) - require.Equal(t, "c8491df826eccf7557467c74f7a93bb324a15efd2359dc27e3eba940127ff8a2", hex.EncodeToString(vk.Bytes())) + require.Equal(t, "f09f918a20696620796f75206172652000000000000000000000000000000000fe570d9ce4722e7021128023dd1251d3145c6ddf8e3a2bc7628b7f802f0d0ff8", hex.EncodeToString(sk.Bytes())) + require.Equal(t, "fe570d9ce4722e7021128023dd1251d3145c6ddf8e3a2bc7628b7f802f0d0ff8", hex.EncodeToString(vk.Bytes())) } func TestVRFKeygenProveAndVerify(t *testing.T) { diff --git a/consensus/pacemaker.go b/consensus/pacemaker.go index 0611775ec..55e685bf1 100644 --- a/consensus/pacemaker.go +++ b/consensus/pacemaker.go @@ -147,7 +147,7 @@ func (p *paceMaker) ShouldHandleMessage(msg *typesCons.HotstuffMessage) (bool, e // Message is from the past if msg.Round < currentRound || (msg.Round == currentRound && msg.Step < currentStep) { - p.consensusMod.nodeLog(fmt.Sprintf("[WARN][DISCARDING] Node at (step, round) (%d, %d, %d) received message at (%d, %d, %d)", currentHeight, currentRound, currentHeight, currentStep, msg.Round, msg.Step)) + p.consensusMod.nodeLog(fmt.Sprintf("[WARN][DISCARDING] Node at (height, step, round) (%d, %d, %d) received message at (%d, %d, %d)", currentHeight, currentStep, currentRound, msg.Height, msg.Step, msg.Round)) return false, nil } diff --git a/consensus/types/errors.go b/consensus/types/errors.go index e08d62a49..d60b2b268 100644 --- a/consensus/types/errors.go +++ b/consensus/types/errors.go @@ -20,12 +20,12 @@ const ( ProposalBlockExtends = "the ProposalQC block is the same as the LockedQC block" // WARN - NilUtilityContextWarning = "[WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it" - InvalidPartialSigInQCWarning = "[WARN] QC contains an invalid partial signature" + NilUtilityContextWarning = "⚠️ [WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it" + InvalidPartialSigInQCWarning = "⚠️ [WARN] QC contains an invalid partial signature" // DEBUG - DebugResetToGenesis = "[DEBUG] Resetting to genesis..." - DebugTriggerNextView = "[DEBUG] Triggering next view..." + DebugResetToGenesis = "🧑‍💻 [DEVELOP] Resetting to genesis..." + DebugTriggerNextView = "🧑‍💻 [DEVELOP] Triggering next view..." ) var StepToString map[HotstuffStep]string @@ -43,47 +43,47 @@ func init() { // 3. Remove this file and move log related text into place (easier to maintain, debug, understand, etc.) func PacemakerInterrupt(reason string, height uint64, step HotstuffStep, round uint64) string { - return fmt.Sprintf("INTERRUPT at (height, step, round): (%d, %s, %d)! Reason: %s", height, StepToString[step], round, reason) + return fmt.Sprintf("⏰ Interrupt ⏰ at (height, step, round): (%d, %s, %d)! Reason: %s", height, StepToString[step], round, reason) } func PacemakerTimeout(height uint64, step HotstuffStep, round uint64) string { - return fmt.Sprintf("Timed out at (height, step, round) (%d, %s, %d)!", height, StepToString[step], round) + return fmt.Sprintf("⌛ Timed out ⌛ at (height, step, round) (%d, %s, %d)!", height, StepToString[step], round) } func PacemakerNewHeight(height uint64) string { - return fmt.Sprintf("Starting first round for new block at height: %d", height) + return fmt.Sprintf("🏁 Starting 1st round 🏁 for height: %d", height) } func PacemakerCatchup(height1, step1, round1, height2, step2, round2 uint64) string { - return fmt.Sprintf("pacemaker catching up the node's (height, step, round) FROM (%d, %s, %d) TO (%d, %s, %d)", height1, StepToString[HotstuffStep(step1)], round1, height2, StepToString[HotstuffStep(step2)], round2) + return fmt.Sprintf("🏃 Pacemaker catching 🏃 up (height, step, round) FROM (%d, %d, %d) TO (%d, %d, %d)", height1, step1, round1, height2, step2, round2) } func OptimisticVoteCountWaiting(step HotstuffStep, status string) string { - return fmt.Sprintf("Waiting for more %s messages; %s", StepToString[step], status) + return fmt.Sprintf("⏳ Waiting ⏳for more %s messages; %s", StepToString[step], status) } -func OptimisticVoteCountPassed(step HotstuffStep) string { - return fmt.Sprintf("received enough %s votes!", StepToString[step]) +func OptimisticVoteCountPassed(height uint64, step HotstuffStep, round uint64) string { + return fmt.Sprintf("📬 Received enough 📬 votes at (height, step, round) (%d, %s, %d)", height, StepToString[step], round) } func CommittingBlock(height uint64, numTxs int) string { - return fmt.Sprintf("🧱🧱🧱 Committing block at height %d with %d transactions 🧱🧱🧱", height, numTxs) + return fmt.Sprintf("🧱 Committing block 🧱 at height %d with %d transactions", height, numTxs) } func ElectedNewLeader(address string, nodeId NodeId, height, round uint64) string { - return fmt.Sprintf("🙇🙇🙇 Elected leader for height/round %d/%d: [%d] (%s) 🙇🙇🙇", height, round, nodeId, address) + return fmt.Sprintf("🙇 Elected leader 🙇 for height/round %d/%d: [%d] (%s)", height, round, nodeId, address) } func ElectedSelfAsNewLeader(address string, nodeId NodeId, height, round uint64) string { - return fmt.Sprintf("👑👑👑 I am the leader for height/round %d/%d: [%d] (%s) 👑👑👑", height, round, nodeId, address) + return fmt.Sprintf("👑 I am the leader 👑 for height/round %d/%d: [%d] (%s)", height, round, nodeId, address) } func SendingMessage(msg *HotstuffMessage, nodeId NodeId) string { - return fmt.Sprintf("Sending %s message to %d", StepToString[msg.GetStep()], nodeId) + return fmt.Sprintf("✉️ Sending message ✉️ to %d at (height, step, round) (%d, %d, %d)", nodeId, msg.Height, msg.Step, msg.Round) } func BroadcastingMessage(msg *HotstuffMessage) string { - return fmt.Sprintf("Broadcasting message for %s step", StepToString[msg.GetStep()]) + return fmt.Sprintf("📣 Broadcasting message 📣 (height, step, round): (%d, %d, %d)", msg.GetHeight(), msg.GetStep(), msg.GetRound()) } func WarnInvalidPartialSigInQC(address string, nodeId NodeId) string { @@ -91,37 +91,37 @@ func WarnInvalidPartialSigInQC(address string, nodeId NodeId) string { } func WarnMissingPartialSig(msg *HotstuffMessage) string { - return fmt.Sprintf("[WARN] No partial signature found for step %s which should not happen...", StepToString[msg.GetStep()]) + return fmt.Sprintf("⚠️ [WARN] No partial signature found for step %s which should not happen...", StepToString[msg.GetStep()]) } func WarnDiscardHotstuffMessage(_ *HotstuffMessage, reason string) string { - return fmt.Sprintf("[WARN] %s because: %s", DisregardHotstuffMessage, reason) + return fmt.Sprintf("⚠️ [WARN] %s because: %s", DisregardHotstuffMessage, reason) } func WarnUnexpectedMessageInPool(_ *HotstuffMessage, height uint64, step HotstuffStep, round uint64) string { - return fmt.Sprintf("[WARN] Message in pool does not match (height, step, round) of QC being generated; %d, %s, %d", height, StepToString[step], round) + return fmt.Sprintf("⚠️ [WARN] Message in pool does not match (height, step, round) of QC being generated; %d, %s, %d", height, StepToString[step], round) } func WarnIncompletePartialSig(ps *PartialSignature, msg *HotstuffMessage) string { - return fmt.Sprintf("[WARN] Partial signature is incomplete for step %s which should not happen...", StepToString[msg.GetStep()]) + return fmt.Sprintf("⚠️ [WARN] Partial signature is incomplete for step %s which should not happen...", StepToString[msg.GetStep()]) } func DebugTogglePacemakerManualMode(mode string) string { - return fmt.Sprintf("[DEBUG] Toggling pacemaker manual mode to %s", mode) + return fmt.Sprintf("🔎 [DEBUG] Toggling pacemaker manual mode to %s", mode) } func DebugNodeState(state ConsensusNodeState) string { - return fmt.Sprintf("[DEBUG] Node %d is at (Height, Step, Round): (%d, %d, %d)\n", state.NodeId, state.Height, state.Step, state.Round) + return fmt.Sprintf("🔎 [DEBUG] Node %d is at (Height, Step, Round): (%d, %d, %d)", state.NodeId, state.Height, state.Step, state.Round) } // TODO(olshansky): Add source and destination NodeId of message here func DebugReceivedHandlingHotstuffMessage(msg *HotstuffMessage) string { - return fmt.Sprintf("[DEBUG] Received hotstuff msg at (Height, Step, Round): (%d, %d, %d)\n", msg.Height, msg.GetStep(), msg.Round) + return fmt.Sprintf("🔎 [DEBUG] Received hotstuff msg at (Height, Step, Round): (%d, %d, %d)", msg.Height, msg.GetStep(), msg.Round) } // TODO(olshansky): Add source and destination NodeId of message here func DebugHandlingHotstuffMessage(msg *HotstuffMessage) string { - return fmt.Sprintf("[DEBUG] Handling hotstuff msg at (Height, Step, Round): (%d, %d, %d)\n", msg.Height, msg.GetStep(), msg.Round) + return fmt.Sprintf("🔎 [DEBUG] Handling hotstuff msg at (Height, Step, Round): (%d, %d, %d)", msg.Height, msg.GetStep(), msg.Round) } // Errors From f1232f1446f51b9e404d34702fdd48171e7a211f Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Fri, 30 Dec 2022 21:22:35 -0500 Subject: [PATCH 07/21] Rename testChannel to eventsChannel --- consensus/e2e_tests/hotstuff_test.go | 22 ++++++------- consensus/e2e_tests/pacemaker_test.go | 20 ++++++------ consensus/e2e_tests/utils_test.go | 46 +++++++++++++-------------- consensus/module.go | 2 +- p2p/utils_test.go | 8 ++--- 5 files changed, 49 insertions(+), 49 deletions(-) diff --git a/consensus/e2e_tests/hotstuff_test.go b/consensus/e2e_tests/hotstuff_test.go index 24e610c3a..0f01a0265 100644 --- a/consensus/e2e_tests/hotstuff_test.go +++ b/consensus/e2e_tests/hotstuff_test.go @@ -20,8 +20,8 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { go timeReminder(t, clockMock, 100*time.Millisecond) // Create & start test pocket nodes - testChannel := make(modules.EventsChannel, 100) - pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel) + eventsChannel := make(modules.EventsChannel, 100) + pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, eventsChannel) StartAllTestPocketNodes(t, pocketNodes) // Debug message to start consensus by triggering first view change @@ -32,7 +32,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // NewRound - newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 1000) + newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 1000) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -57,7 +57,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Prepare - prepareProposal, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 1000) + prepareProposal, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Prepare, consensus.Propose, 1, 1000) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -77,7 +77,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Precommit - prepareVotes, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Vote, numValidators, 1000) + prepareVotes, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Prepare, consensus.Vote, numValidators, 1000) require.NoError(t, err) for _, vote := range prepareVotes { P2PSend(t, leader, vote) @@ -85,7 +85,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) - preCommitProposal, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.PreCommit, consensus.Propose, 1, 1000) + preCommitProposal, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.PreCommit, consensus.Propose, 1, 1000) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -105,7 +105,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Commit - preCommitVotes, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.PreCommit, consensus.Vote, numValidators, 1000) + preCommitVotes, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.PreCommit, consensus.Vote, numValidators, 1000) require.NoError(t, err) for _, vote := range preCommitVotes { P2PSend(t, leader, vote) @@ -113,7 +113,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) - commitProposal, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Commit, consensus.Propose, 1, 1000) + commitProposal, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Commit, consensus.Propose, 1, 1000) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -133,7 +133,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Decide - commitVotes, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Commit, consensus.Vote, numValidators, 1000) + commitVotes, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Commit, consensus.Vote, numValidators, 1000) require.NoError(t, err) for _, vote := range commitVotes { P2PSend(t, leader, vote) @@ -141,7 +141,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) - decideProposal, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Decide, consensus.Propose, 1, 1000) + decideProposal, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Decide, consensus.Propose, 1, 1000) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -173,7 +173,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Block has been committed and new round has begun - _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 1000) + _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 1000) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) diff --git a/consensus/e2e_tests/pacemaker_test.go b/consensus/e2e_tests/pacemaker_test.go index 045dfe075..f40fff9af 100644 --- a/consensus/e2e_tests/pacemaker_test.go +++ b/consensus/e2e_tests/pacemaker_test.go @@ -30,8 +30,8 @@ func TestTinyPacemakerTimeouts(t *testing.T) { } // Create & start test pocket nodes - testChannel := make(modules.EventsChannel, 100) - pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel) + eventsChannel := make(modules.EventsChannel, 100) + pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, eventsChannel) StartAllTestPocketNodes(t, pocketNodes) // // Debug message to start consensus by triggering next view. @@ -43,7 +43,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // paceMakerTimeout - _, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) + _, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 500) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -58,7 +58,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { forcePacemakerTimeout(t, clockMock, paceMakerTimeout) // Check that a new round starts at the same height. - _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) + _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 500) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -73,7 +73,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { forcePacemakerTimeout(t, clockMock, paceMakerTimeout) // Check that a new round starts at the same height - _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) + _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 500) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -88,7 +88,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { forcePacemakerTimeout(t, clockMock, paceMakerTimeout) // Check that a new round starts at the same height. - newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 500) + newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 500) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -110,7 +110,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Confirm we are at the next step - _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 500) + _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Prepare, consensus.Propose, 1, 500) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -130,8 +130,8 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { timeReminder(t, clockMock, 100*time.Millisecond) // Create & start test pocket nodes - testChannel := make(modules.EventsChannel, 100) - pocketNodes := CreateTestConsensusPocketNodes(t, runtimeConfigs, testChannel) + eventsChannel := make(modules.EventsChannel, 100) + pocketNodes := CreateTestConsensusPocketNodes(t, runtimeConfigs, eventsChannel) StartAllTestPocketNodes(t, pocketNodes) // Starting point @@ -197,7 +197,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { P2PBroadcast(t, pocketNodes, anyMsg) // numNodes-1 because one of the messages is a self-proposal that is not passed through the network - _, err = WaitForNetworkConsensusMessages(t, clockMock, testChannel, consensus.Prepare, consensus.Vote, numValidators-1, 2000) + _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Prepare, consensus.Vote, numValidators-1, 2000) require.NoError(t, err) forcePacemakerTimeout(t, clockMock, 600*time.Millisecond) diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index 0bde7d022..68460d62d 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -85,7 +85,7 @@ func GenerateNodeRuntimeMgrs(_ *testing.T, validatorCount int, clockMgr clock.Cl func CreateTestConsensusPocketNodes( t *testing.T, runtimeMgrs []runtime.Manager, - testChannel modules.EventsChannel, + eventsChannel modules.EventsChannel, ) (pocketNodes IdToNodeMapping) { pocketNodes = make(IdToNodeMapping, len(runtimeMgrs)) // TODO(design): The order here is important in order for NodeId to be set correctly below. @@ -98,7 +98,7 @@ func CreateTestConsensusPocketNodes( return pk.Address().String() < pk2.Address().String() }) for i, runtimeMgr := range runtimeMgrs { - pocketNode := CreateTestConsensusPocketNode(t, &runtimeMgr, testChannel) + pocketNode := CreateTestConsensusPocketNode(t, &runtimeMgr, eventsChannel) // TODO(olshansky): Figure this part out. pocketNodes[typesCons.NodeId(i+1)] = pocketNode } @@ -108,7 +108,7 @@ func CreateTestConsensusPocketNodes( func CreateTestConsensusPocketNodesNew( t *testing.T, runtimeMgrs []runtime.Manager, - testChannel modules.EventsChannel, + eventsChannel modules.EventsChannel, ) (pocketNodes IdToNodeMapping) { pocketNodes = make(IdToNodeMapping, len(runtimeMgrs)) // TODO(design): The order here is important in order for NodeId to be set correctly below. @@ -121,7 +121,7 @@ func CreateTestConsensusPocketNodesNew( return pk.Address().String() < pk2.Address().String() }) for i, runtimeMgr := range runtimeMgrs { - pocketNode := CreateTestConsensusPocketNode(t, &runtimeMgr, testChannel) + pocketNode := CreateTestConsensusPocketNode(t, &runtimeMgr, eventsChannel) // TODO(olshansky): Figure this part out. pocketNodes[typesCons.NodeId(i+1)] = pocketNode } @@ -132,18 +132,18 @@ func CreateTestConsensusPocketNodesNew( func CreateTestConsensusPocketNode( t *testing.T, runtimeMgr *runtime.Manager, - testChannel modules.EventsChannel, + eventsChannel modules.EventsChannel, ) *shared.Node { consensusMod, err := consensus.Create(runtimeMgr) require.NoError(t, err) // TODO(olshansky): At the moment we are using the same base mocks for all the tests, // but note that they will need to be customized on a per test basis. - persistenceMock := basePersistenceMock(t, testChannel) - p2pMock := baseP2PMock(t, testChannel) - utilityMock := baseUtilityMock(t, testChannel, runtimeMgr.GetGenesis()) - telemetryMock := baseTelemetryMock(t, testChannel) - loggerMock := baseLoggerMock(t, testChannel) - rpcMock := baseRpcMock(t, testChannel) + persistenceMock := basePersistenceMock(t, eventsChannel) + p2pMock := baseP2PMock(t, eventsChannel) + utilityMock := baseUtilityMock(t, eventsChannel, runtimeMgr.GetGenesis()) + telemetryMock := baseTelemetryMock(t, eventsChannel) + loggerMock := baseLoggerMock(t, eventsChannel) + rpcMock := baseRpcMock(t, eventsChannel) bus, err := shared.CreateBus(runtimeMgr, persistenceMock, p2pMock, utilityMock, consensusMod.(modules.ConsensusModule), telemetryMock, loggerMock, rpcMock) @@ -220,7 +220,7 @@ func P2PSend(_ *testing.T, node *shared.Node, any *anypb.Any) { func WaitForNetworkConsensusMessages( t *testing.T, clock *clock.Mock, - testChannel modules.EventsChannel, + eventsChannel modules.EventsChannel, step typesCons.HotstuffStep, msgType typesCons.HotstuffMessageType, numExpectedMsgs int, @@ -237,14 +237,14 @@ func WaitForNetworkConsensusMessages( } errorMessage := fmt.Sprintf("HotStuff step: %s, type: %s", typesCons.HotstuffStep_name[int32(step)], typesCons.HotstuffMessageType_name[int32(msgType)]) - return waitForNetworkConsensusMessagesInternal(t, clock, testChannel, consensus.HotstuffMessageContentType, numExpectedMsgs, millis, includeFilter, errorMessage) + return waitForNetworkConsensusMessagesInternal(t, clock, eventsChannel, consensus.HotstuffMessageContentType, numExpectedMsgs, millis, includeFilter, errorMessage) } // IMPROVE(olshansky): Translate this to use generics. func waitForNetworkConsensusMessagesInternal( t *testing.T, clock *clock.Mock, - testChannel modules.EventsChannel, + eventsChannel modules.EventsChannel, eventContentType string, numExpectedMsgs int, millis time.Duration, @@ -269,15 +269,15 @@ func waitForNetworkConsensusMessagesInternal( loop: for { select { - case testEvent := <-testChannel: - if testEvent.GetContentType() != eventContentType { - unusedEvents = append(unusedEvents, &testEvent) + case nodeEvent := <-eventsChannel: + if nodeEvent.GetContentType() != eventContentType { + unusedEvents = append(unusedEvents, &nodeEvent) continue } - message := testEvent.Content + message := nodeEvent.Content if message == nil || !includeFilter(message) { - unusedEvents = append(unusedEvents, &testEvent) + unusedEvents = append(unusedEvents, &nodeEvent) continue } @@ -300,7 +300,7 @@ loop: } } for _, u := range unusedEvents { - testChannel <- *u + eventsChannel <- *u } return } @@ -331,7 +331,7 @@ func basePersistenceMock(t *testing.T, _ modules.EventsChannel) *modulesMock.Moc } // Creates a p2p module mock with mock implementations of some basic functionality -func baseP2PMock(t *testing.T, testChannel modules.EventsChannel) *modulesMock.MockP2PModule { +func baseP2PMock(t *testing.T, eventsChannel modules.EventsChannel) *modulesMock.MockP2PModule { ctrl := gomock.NewController(t) p2pMock := modulesMock.NewMockP2PModule(ctrl) @@ -341,14 +341,14 @@ func baseP2PMock(t *testing.T, testChannel modules.EventsChannel) *modulesMock.M Broadcast(gomock.Any()). Do(func(msg *anypb.Any) { e := &messaging.PocketEnvelope{Content: msg} - testChannel <- *e + eventsChannel <- *e }). AnyTimes() p2pMock.EXPECT(). Send(gomock.Any(), gomock.Any()). Do(func(addr cryptoPocket.Address, msg *anypb.Any) { e := &messaging.PocketEnvelope{Content: msg} - testChannel <- *e + eventsChannel <- *e }). AnyTimes() diff --git a/consensus/module.go b/consensus/module.go index faabeab4f..064c26747 100644 --- a/consensus/module.go +++ b/consensus/module.go @@ -319,7 +319,7 @@ func (m *consensusModule) loadPersistedState() error { latestHeight, err := persistenceContext.GetLatestBlockHeight() if err != nil || latestHeight == 0 { - m.nodeLog("TODO: State sync not implemented yet") + // TODO: Proper state sync not implemented yet return nil } diff --git a/p2p/utils_test.go b/p2p/utils_test.go index fb279db64..606910b9d 100644 --- a/p2p/utils_test.go +++ b/p2p/utils_test.go @@ -24,7 +24,7 @@ import ( const ( serviceUrlFormat = "node%d.consensus:8080" - testChannelSize = 10000 + eventsChannelSize = 10000 // Since we simulate up to a 27 node network, we will pre-generate a n >= 27 number of keys to avoid generation // every time. The genesis config seed start is set for deterministic key generation and 42 was chosen arbitrarily. genesisConfigSeedStart = 42 @@ -253,18 +253,18 @@ func prepareEventMetricsAgentMock(t *testing.T, wg *sync.WaitGroup, expectedNumN // Network transport mock - used to simulate reading to/from the network via the main events channel // as well as counting the number of expected reads func prepareConnMock(t *testing.T, wg *sync.WaitGroup, expectedNumNetworkReads int) typesP2P.Transport { - testChannel := make(chan []byte, testChannelSize) + eventsChannel := make(chan []byte, eventsChannelSize) ctrl := gomock.NewController(t) connMock := mocksP2P.NewMockTransport(ctrl) connMock.EXPECT().Read().DoAndReturn(func() ([]byte, error) { wg.Done() - data := <-testChannel + data := <-eventsChannel return data, nil }).Times(expectedNumNetworkReads + 1) // +1 is necessary because there is one extra read of empty data by every channel when it starts connMock.EXPECT().Write(gomock.Any()).DoAndReturn(func(data []byte) error { - testChannel <- data + eventsChannel <- data return nil }).Times(expectedNumNetworkReads) From 728388aa3f0ca420adfc59cb343f534eef13783c Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 2 Jan 2023 13:25:50 -0500 Subject: [PATCH 08/21] Make TestTinyPacemakerTimeouts pass and fix how we fail on extra messages --- Makefile | 12 +-- consensus/e2e_tests/hotstuff_test.go | 24 +++--- consensus/e2e_tests/pacemaker_test.go | 43 ++++++----- consensus/e2e_tests/utils_test.go | 103 +++++++++++++------------- shared/bus.go | 4 +- shared/modules/bus_module.go | 2 +- 6 files changed, 94 insertions(+), 94 deletions(-) diff --git a/Makefile b/Makefile index 0e81792b1..54f462f77 100644 --- a/Makefile +++ b/Makefile @@ -2,14 +2,6 @@ include build.mk CWD ?= CURRENT_WORKING_DIRECTIONRY_NOT_SUPPLIED -# This flag is useful when running the consensus unit tests. It causes the test to wait up to the -# maximum delay specified in the source code and errors if additional unexpected messages are received. -# For example, if the test expects to receive 5 messages within 2 seconds: -# When EXTRA_MSG_FAIL = false: continue if 5 messages are received in 0.5 seconds -# When EXTRA_MSG_FAIL = true: wait for another 1.5 seconds after 5 messages are received in 0.5 -# seconds, and fail if any additional messages are received. -EXTRA_MSG_FAIL ?= false - # IMPROVE: Add `-shuffle=on` to the `go test` command to randomize the order in which tests are run. # An easy way to turn off verbose test output for some of the test targets. For example @@ -323,11 +315,11 @@ test_consensus_concurrent_tests: ## Run unit tests in the consensus module that .PHONY: test_hotstuff test_hotstuff: ## Run all go unit tests related to hotstuff consensus - go test ${VERBOSE_TEST} ./consensus/e2e_tests -run Hotstuff -failOnExtraMessages=${EXTRA_MSG_FAIL} + go test ${VERBOSE_TEST} ./consensus/e2e_tests -run Hotstuff .PHONY: test_pacemaker test_pacemaker: ## Run all go unit tests related to the hotstuff pacemaker - go test ${VERBOSE_TEST} ./consensus/e2e_tests -run Pacemaker -failOnExtraMessages=${EXTRA_MSG_FAIL} + go test ${VERBOSE_TEST} ./consensus/e2e_tests -run Pacemaker .PHONY: test_vrf test_vrf: ## Run all go unit tests in the VRF library diff --git a/consensus/e2e_tests/hotstuff_test.go b/consensus/e2e_tests/hotstuff_test.go index 0f01a0265..3ec9036e7 100644 --- a/consensus/e2e_tests/hotstuff_test.go +++ b/consensus/e2e_tests/hotstuff_test.go @@ -17,11 +17,11 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { // Test configs runtimeMgrs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) - go timeReminder(t, clockMock, 100*time.Millisecond) + go timeReminder(t, clockMock, time.Second) // Create & start test pocket nodes - eventsChannel := make(modules.EventsChannel, 100) - pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, eventsChannel) + testChannel := make(modules.EventsChannel, 100) + pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel) StartAllTestPocketNodes(t, pocketNodes) // Debug message to start consensus by triggering first view change @@ -32,7 +32,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // NewRound - newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 1000) + newRoundMessages, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 1000, false) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -57,7 +57,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Prepare - prepareProposal, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Prepare, consensus.Propose, 1, 1000) + prepareProposal, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 1000, false) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -77,7 +77,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Precommit - prepareVotes, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Prepare, consensus.Vote, numValidators, 1000) + prepareVotes, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Prepare, consensus.Vote, numValidators, 1000, false) require.NoError(t, err) for _, vote := range prepareVotes { P2PSend(t, leader, vote) @@ -85,7 +85,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) - preCommitProposal, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.PreCommit, consensus.Propose, 1, 1000) + preCommitProposal, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.PreCommit, consensus.Propose, 1, 1000, false) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -105,7 +105,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Commit - preCommitVotes, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.PreCommit, consensus.Vote, numValidators, 1000) + preCommitVotes, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.PreCommit, consensus.Vote, numValidators, 1000, false) require.NoError(t, err) for _, vote := range preCommitVotes { P2PSend(t, leader, vote) @@ -113,7 +113,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) - commitProposal, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Commit, consensus.Propose, 1, 1000) + commitProposal, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Commit, consensus.Propose, 1, 1000, false) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -133,7 +133,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Decide - commitVotes, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Commit, consensus.Vote, numValidators, 1000) + commitVotes, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Commit, consensus.Vote, numValidators, 1000, false) require.NoError(t, err) for _, vote := range commitVotes { P2PSend(t, leader, vote) @@ -141,7 +141,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) - decideProposal, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Decide, consensus.Propose, 1, 1000) + decideProposal, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Decide, consensus.Propose, 1, 1000, false) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -173,7 +173,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { advanceTime(t, clockMock, 10*time.Millisecond) // Block has been committed and new round has begun - _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 1000) + _, err = WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 1000, false) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) diff --git a/consensus/e2e_tests/pacemaker_test.go b/consensus/e2e_tests/pacemaker_test.go index f40fff9af..7f340b65c 100644 --- a/consensus/e2e_tests/pacemaker_test.go +++ b/consensus/e2e_tests/pacemaker_test.go @@ -16,12 +16,16 @@ import ( ) func TestTinyPacemakerTimeouts(t *testing.T) { + // Test preparation clockMock := clock.NewMock() - timeReminder(t, clockMock, 100*time.Millisecond) - - // Test configs - paceMakerTimeoutMsec := uint64(50) // Set a very small pacemaker timeout - paceMakerTimeout := 50 * time.Millisecond + timeReminder(t, clockMock, time.Second) + + // UnitTestNet configs + paceMakerTimeoutMsec := uint64(500) // Set a small pacemaker timeout + paceMakerTimeout := time.Duration(paceMakerTimeoutMsec) * time.Millisecond + // Must be smaller than pacemaker timeout for the test to work properly because we are waiting + // for a deterministic number of consensus messages. + consensusMessageTimeoutMsec := time.Duration(paceMakerTimeoutMsec / 5) runtimeMgrs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) for _, runtimeConfig := range runtimeMgrs { if consCfg, ok := runtimeConfig.GetConfig().GetConsensusConfig().(consensus.HasPacemakerConfig); ok { @@ -34,17 +38,18 @@ func TestTinyPacemakerTimeouts(t *testing.T) { pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, eventsChannel) StartAllTestPocketNodes(t, pocketNodes) - // // Debug message to start consensus by triggering next view. + // Debug message to start consensus by triggering next view for _, pocketNode := range pocketNodes { TriggerNextView(t, pocketNode) } - // advance time by an amount shorter than the timeout + // Advance time by an amount shorter than the pacemaker timeout advanceTime(t, clockMock, 10*time.Millisecond) - // paceMakerTimeout - _, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 500) + // Verify consensus started - NewRound messages have an N^2 complexity + _, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators*numValidators, consensusMessageTimeoutMsec, true) require.NoError(t, err) + for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, typesCons.ConsensusNodeState{ @@ -55,10 +60,11 @@ func TestTinyPacemakerTimeouts(t *testing.T) { GetConsensusNodeState(pocketNode)) } + // // Force the pacemaker to time out forcePacemakerTimeout(t, clockMock, paceMakerTimeout) - // Check that a new round starts at the same height. - _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 500) + // Verify that a new round started at the same height + _, err = WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators*numValidators, consensusMessageTimeoutMsec, true) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -73,7 +79,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { forcePacemakerTimeout(t, clockMock, paceMakerTimeout) // Check that a new round starts at the same height - _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 500) + _, err = WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators*numValidators, consensusMessageTimeoutMsec, true) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -88,7 +94,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { forcePacemakerTimeout(t, clockMock, paceMakerTimeout) // Check that a new round starts at the same height. - newRoundMessages, err := WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators, 500) + newRoundMessages, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators*numValidators, consensusMessageTimeoutMsec, true) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -102,15 +108,14 @@ func TestTinyPacemakerTimeouts(t *testing.T) { // Continue to the next step at the current round for _, message := range newRoundMessages { - // msg, _ := codec.GetCodec().FromAny(message) P2PBroadcast(t, pocketNodes, message) } // advance time by an amount shorter than the timeout advanceTime(t, clockMock, 10*time.Millisecond) - // Confirm we are at the next step - _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Prepare, consensus.Propose, 1, 500) + // Confirm we are at the next step (NewRound -> Prepare) + _, err = WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Prepare, consensus.Propose, numValidators, consensusMessageTimeoutMsec, true) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { assertNodeConsensusView(t, pocketId, @@ -127,7 +132,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { clockMock := clock.NewMock() runtimeConfigs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) - timeReminder(t, clockMock, 100*time.Millisecond) + timeReminder(t, clockMock, time.Second) // Create & start test pocket nodes eventsChannel := make(modules.EventsChannel, 100) @@ -157,7 +162,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { } block := &typesCons.Block{ BlockHeader: blockHeader, - Transactions: emptyTxs, + Transactions: make([][]byte, 0), } leaderConsensusModImpl := GetConsensusModImpl(leader) @@ -197,7 +202,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { P2PBroadcast(t, pocketNodes, anyMsg) // numNodes-1 because one of the messages is a self-proposal that is not passed through the network - _, err = WaitForNetworkConsensusMessages(t, clockMock, eventsChannel, consensus.Prepare, consensus.Vote, numValidators-1, 2000) + _, err = WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Prepare, consensus.Vote, numValidators-1, 2000, false) require.NoError(t, err) forcePacemakerTimeout(t, clockMock, 600*time.Millisecond) diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index 68460d62d..992854bea 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -2,9 +2,7 @@ package e2e_tests import ( "context" - "flag" "fmt" - "log" "os" "reflect" "sort" @@ -32,32 +30,14 @@ func TestMain(m *testing.M) { os.Exit(exitCode) } -// If this is set to true, consensus unit tests will fail if additional unexpected messages are received. -// This slows down the tests because we always fail until the timeout specified by the test before continuing -// but guarantees more correctness. -var failOnExtraMessages bool - // TODO(integration): These are temporary variables used in the prototype integration phase that // will need to be parameterized later once the test framework design matures. -var ( - stateHash = "42" - maxTxBytes = 90000 - emptyByzValidators = make([][]byte, 0) - emptyTxs = make([][]byte, 0) +const ( + numValidators = 4 + stateHash = "42" + maxTxBytes = 90000 ) -const numValidators = 4 - -// Initialize certain unit test configurations on startup. -func init() { - flag.BoolVar(&failOnExtraMessages, "failOnExtraMessages", false, "Fail if unexpected additional messages are received") - - var err error - if err != nil { - log.Fatalf(err.Error()) - } -} - type IdToNodeMapping map[typesCons.NodeId]*shared.Node /*** Node Generation Helpers ***/ @@ -217,7 +197,15 @@ func P2PSend(_ *testing.T, node *shared.Node, any *anypb.Any) { node.GetBus().PublishEventToBus(e) } -func WaitForNetworkConsensusMessages( +// This is a helper for `waitForEventsInternal` that creates the `includeFilter` function based on +// consensus specific parameters. +// failOnExtraMessages: +// This flag is useful when running the consensus unit tests. It causes the test to wait up to the +// maximum delay specified in the source code and errors if additional unexpected messages are received. +// For example, if the test expects to receive 5 messages within 2 seconds: +// false: continue if 5 messages are received in 0.5 seconds +// true: true: wait for another 1.5 seconds after 5 messages are received in 0.5 seconds, and fail if any additional messages are received. +func WaitForNetworkConsensusEvents( t *testing.T, clock *clock.Mock, eventsChannel modules.EventsChannel, @@ -225,6 +213,7 @@ func WaitForNetworkConsensusMessages( msgType typesCons.HotstuffMessageType, numExpectedMsgs int, millis time.Duration, + failOnExtraMessages bool, ) (messages []*anypb.Any, err error) { includeFilter := func(anyMsg *anypb.Any) bool { msg, err := codec.GetCodec().FromAny(anyMsg) @@ -233,74 +222,88 @@ func WaitForNetworkConsensusMessages( hotstuffMessage, ok := msg.(*typesCons.HotstuffMessage) require.True(t, ok) + fmt.Println("OLSH", hotstuffMessage.Round, hotstuffMessage.Step, hotstuffMessage.Type) + return hotstuffMessage.Type == msgType && hotstuffMessage.Step == step } - errorMessage := fmt.Sprintf("HotStuff step: %s, type: %s", typesCons.HotstuffStep_name[int32(step)], typesCons.HotstuffMessageType_name[int32(msgType)]) - return waitForNetworkConsensusMessagesInternal(t, clock, eventsChannel, consensus.HotstuffMessageContentType, numExpectedMsgs, millis, includeFilter, errorMessage) + errMsg := fmt.Sprintf("HotStuff step: %s, type: %s", typesCons.HotstuffStep_name[int32(step)], typesCons.HotstuffMessageType_name[int32(msgType)]) + return waitForEventsInternal(t, clock, eventsChannel, consensus.HotstuffMessageContentType, numExpectedMsgs, millis, includeFilter, errMsg, failOnExtraMessages) } -// IMPROVE(olshansky): Translate this to use generics. -func waitForNetworkConsensusMessagesInternal( +// IMPROVE: This function can be extended to testing events outside of just the consensus module. +func waitForEventsInternal( t *testing.T, clock *clock.Mock, eventsChannel modules.EventsChannel, eventContentType string, numExpectedMsgs int, - millis time.Duration, - includeFilter func(m *anypb.Any) bool, - errorMsg string, + maxWaitTimeMillis time.Duration, + msgIncludeFilter func(m *anypb.Any) bool, + errMsg string, + failOnExtraMessages bool, ) (expectedMsgs []*anypb.Any, err error) { expectedMsgs = make([]*anypb.Any, 0) // Aggregate and return the messages we're waiting for unusedEvents := make([]*messaging.PocketEnvelope, 0) // "Recycle" events back into the events channel if we're not using them // Limit the amount of time we're waiting for the messages to be published on the events channel - ctx, cancel := clock.WithTimeout(context.TODO(), time.Millisecond*millis) + ctx, cancel := clock.WithTimeout(context.TODO(), time.Millisecond*maxWaitTimeMillis) defer cancel() // Since the tests use a mock clock, we need to manually advance the clock to trigger the timeout - ticker := clock.Ticker(time.Millisecond) + ticker := time.NewTicker(time.Millisecond) + tickerDone := make(chan bool) go func() { for { - <-ticker.C - clock.Add(time.Millisecond) + select { + case <-tickerDone: + return + case <-ticker.C: + clock.Add(time.Millisecond) + } } }() + + numRemainingMsgs := numExpectedMsgs loop: for { select { case nodeEvent := <-eventsChannel: if nodeEvent.GetContentType() != eventContentType { - unusedEvents = append(unusedEvents, &nodeEvent) + unusedEvents = append(unusedEvents, nodeEvent) continue } message := nodeEvent.Content - if message == nil || !includeFilter(message) { - unusedEvents = append(unusedEvents, &nodeEvent) + if message == nil || !msgIncludeFilter(message) { + unusedEvents = append(unusedEvents, nodeEvent) continue } expectedMsgs = append(expectedMsgs, message) - numExpectedMsgs-- - // Break if: + numRemainingMsgs-- + // Break if both of the following are true: // 1. We are not expecting any more messages - // 2. We do not want to fail on extra messages - if numExpectedMsgs == 0 && !failOnExtraMessages { + // 2. We do not want to fail in the case of extra unexpected messages that pass the filter + if numRemainingMsgs == 0 && !failOnExtraMessages { break loop } case <-ctx.Done(): - if numExpectedMsgs == 0 { + fmt.Println("numRemainingMsgs", numRemainingMsgs) + if numRemainingMsgs == 0 { break loop - } else if numExpectedMsgs > 0 { - t.Fatalf("Missing %s messages; missing: %d, received: %d; (%s)", eventContentType, numExpectedMsgs, len(expectedMsgs), errorMsg) + } else if numRemainingMsgs > 0 { + t.Fatalf("Missing '%s' messages; %d expected but %d received. (%s)", eventContentType, numExpectedMsgs, len(expectedMsgs), errMsg) } else { - t.Fatalf("Too many %s messages received; expected: %d, received: %d; (%s)", eventContentType, numExpectedMsgs+len(expectedMsgs), len(expectedMsgs), errorMsg) + t.Fatalf("Too many '%s' messages; %d expected but %d received. (%s)", eventContentType, numExpectedMsgs, len(expectedMsgs), errMsg) } } } + ticker.Stop() + tickerDone <- true + for _, u := range unusedEvents { - eventsChannel <- *u + eventsChannel <- u } return } @@ -341,14 +344,14 @@ func baseP2PMock(t *testing.T, eventsChannel modules.EventsChannel) *modulesMock Broadcast(gomock.Any()). Do(func(msg *anypb.Any) { e := &messaging.PocketEnvelope{Content: msg} - eventsChannel <- *e + eventsChannel <- e }). AnyTimes() p2pMock.EXPECT(). Send(gomock.Any(), gomock.Any()). Do(func(addr cryptoPocket.Address, msg *anypb.Any) { e := &messaging.PocketEnvelope{Content: msg} - eventsChannel <- *e + eventsChannel <- e }). AnyTimes() diff --git a/shared/bus.go b/shared/bus.go index 951e16cae..ab9b54cfa 100644 --- a/shared/bus.go +++ b/shared/bus.go @@ -129,12 +129,12 @@ func CreateBusWithOptionalModules( } func (m *bus) PublishEventToBus(e *messaging.PocketEnvelope) { - m.channel <- *e + m.channel <- e } func (m *bus) GetBusEvent() *messaging.PocketEnvelope { e := <-m.channel - return &e + return e } func (m *bus) GetEventBus() modules.EventsChannel { diff --git a/shared/modules/bus_module.go b/shared/modules/bus_module.go index 981282f49..ea8fbbb93 100644 --- a/shared/modules/bus_module.go +++ b/shared/modules/bus_module.go @@ -9,7 +9,7 @@ import ( // DISCUSS if this channel should be of pointers to PocketEvents or not. Pointers // would avoid doing object copying, but might also be less thread safe if another goroutine changes // it, which could potentially be a feature rather than a bug. -type EventsChannel chan messaging.PocketEnvelope +type EventsChannel chan *messaging.PocketEnvelope type Bus interface { // Bus Events From ef8c05f55b8f9aceb5c6a22d1b9d85874974dd2e Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 2 Jan 2023 13:28:08 -0500 Subject: [PATCH 09/21] Popped some old stashed code --- consensus/e2e_tests/block_proposal_test.go | 104 +++++++++++++++++++++ consensus/e2e_tests/utils_test.go | 3 - consensus/pacemaker.go | 6 +- 3 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 consensus/e2e_tests/block_proposal_test.go diff --git a/consensus/e2e_tests/block_proposal_test.go b/consensus/e2e_tests/block_proposal_test.go new file mode 100644 index 000000000..15e7e38e3 --- /dev/null +++ b/consensus/e2e_tests/block_proposal_test.go @@ -0,0 +1,104 @@ +package e2e_tests + +// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node4.consensus | [00] [NODE][4] [DEBUG] Triggering next view... +// node4.consensus | [00] [NODE][4] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 0)! Reason: manual trigger +// node4.consensus | [00] [NODE][4] Broadcasting message for HOTSTUFF_STEP_NEWROUND step +// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node3.consensus | [00] [NODE][3] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node1.consensus | [00] [NODE][1] [DEBUG] Triggering next view... +// node1.consensus | [00] [NODE][1] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 0)! Reason: manual trigger +// node1.consensus | [00] [NODE][1] Broadcasting message for HOTSTUFF_STEP_NEWROUND step +// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node3.consensus | [00] [NODE][3] pacemaker catching up the node's (height, step, round) FROM (12, HOTSTUFF_STEP_NEWROUND, 0) TO (12, HOTSTUFF_STEP_NEWROUND, 1) +// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node4.consensus | [00] [NODE][4] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node1.consensus | [00] [NODE][1] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node2.consensus | [00] [NODE][2] [DEBUG] Triggering next view... +// node2.consensus | [00] [NODE][2] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 0)! Reason: manual trigger +// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node2.consensus | [00] [NODE][2] Broadcasting message for HOTSTUFF_STEP_NEWROUND step +// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node2.consensus | [00] [NODE][2] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node4.consensus | [00] [REPLICA][4] 🙇🙇🙇 Elected leader for height/round 12/1: [2] (3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99) 🙇🙇🙇 +// node4.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_REPLICA +// node3.consensus | [00] [REPLICA][3] 🙇🙇🙇 Elected leader for height/round 12/1: [2] (3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99) 🙇🙇🙇 +// node3.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_REPLICA +// node1.consensus | [00] [REPLICA][1] 🙇🙇🙇 Elected leader for height/round 12/1: [2] (3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99) 🙇🙇🙇 +// node2.consensus | [00] [LEADER][2] 👑👑👑 I am the leader for height/round 12/1: [2] (3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99) 👑👑👑 +// node1.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_REPLICA +// node2.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER +// node3.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER +// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node2.consensus | [00] [LEADER][2] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (1 > 2.67?) +// node2.consensus | [00] [LEADER][2] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node3.consensus | [00] [REPLICA][3] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (1 > 2.67?) +// node4.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER +// node2.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER +// node2.consensus | [00] [LEADER][2] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (2 > 2.67?) +// node1.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER +// node3.consensus | [00] [REPLICA][3] [DEBUG] Triggering next view... +// node4.consensus | [00] [REPLICA][4] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (1 > 2.67?) +// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 +// node1.consensus | [00] [REPLICA][1] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (1 > 2.67?) +// node1.consensus | [00] [REPLICA][1] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node1.consensus | [00] [REPLICA][1] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 2. +// node1.consensus | [00] [REPLICA][1] pacemaker catching up the node's (height, step, round) FROM (12, HOTSTUFF_STEP_PREPARE, 1) TO (12, HOTSTUFF_STEP_NEWROUND, 2) +// node4.consensus | [00] [REPLICA][4] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node3.consensus | [00] [REPLICA][3] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_PREPARE, 1)! Reason: manual trigger +// node1.consensus | [00] [REPLICA][1] 🙇🙇🙇 Elected leader for height/round 12/2: [3] (67eb3f0a50ae459fecf666be0e93176e92441317) 🙇🙇🙇 +// node2.consensus | [00] [LEADER][2] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 2. +// node4.consensus | [00] [REPLICA][4] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 2. +// node3.consensus | [00] About to release postgres context at height 12. +// node2.consensus | [00] [LEADER][2] pacemaker catching up the node's (height, step, round) FROM (12, HOTSTUFF_STEP_NEWROUND, 1) TO (12, HOTSTUFF_STEP_NEWROUND, 2) +// node2.consensus | [00] [REPLICA][2] 🙇🙇🙇 Elected leader for height/round 12/2: [3] (67eb3f0a50ae459fecf666be0e93176e92441317) 🙇🙇🙇 +// node3.consensus | [00] [NODE][3] Broadcasting message for HOTSTUFF_STEP_NEWROUND step +// node4.consensus | [00] [REPLICA][4] pacemaker catching up the node's (height, step, round) FROM (12, HOTSTUFF_STEP_PREPARE, 1) TO (12, HOTSTUFF_STEP_NEWROUND, 2) +// node1.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_REPLICA +// node1.consensus | [00] [REPLICA][1] [WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it +// node1.consensus | [00] About to release postgres context at height 12. +// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node2.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_REPLICA +// node1.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_LEADER +// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node4.consensus | [00] [REPLICA][4] 🙇🙇🙇 Elected leader for height/round 12/2: [3] (67eb3f0a50ae459fecf666be0e93176e92441317) 🙇🙇🙇 +// node4.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_REPLICA +// node4.consensus | [00] [REPLICA][4] [WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it +// node1.consensus | [00] [REPLICA][1] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (2 > 2.67?) +// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send +// node4.consensus | [00] About to release postgres context at height 12. +// node2.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_LEADER +// node3.consensus | [00] [NODE][3] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node3.consensus | [00] [NODE][3] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. +// node4.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_LEADER +// node2.consensus | [00] [REPLICA][2] received enough HOTSTUFF_STEP_NEWROUND votes! +// node4.consensus | [00] [REPLICA][4] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (2 > 2.67?) +// node2.consensus | [00] [REPLICA][2] [WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it +// node2.consensus | [00] About to release postgres context at height 12. +// node2.consensus | [00] [REPLICA][2] Preparing a new block - no highPrepareQC found +// node2.consensus | [00] [ERROR][REPLICA][2] could not prepare block: node should not call `prepareBlock` if it is not a leader +// node2.consensus | [00] [REPLICA][2] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_PREPARE, 2)! Reason: failed to prepare & apply block +// node2.consensus | [00] About to release postgres context at height 12. +// node3.consensus | [00] [NODE][3] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 2)! Reason: timeout +// node4.consensus | [00] [REPLICA][4] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_PREPARE, 2)! Reason: timeout +// node4.consensus | [00] About to release postgres context at height 12. +// node1.consensus | [00] [REPLICA][1] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_PREPARE, 2)! Reason: timeout +// node1.consensus | [00] About to release postgres context at height 12. +// node2.consensus | [00] [NODE][2] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 3)! Reason: timeout diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index 992854bea..bf72a5c7a 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -222,8 +222,6 @@ func WaitForNetworkConsensusEvents( hotstuffMessage, ok := msg.(*typesCons.HotstuffMessage) require.True(t, ok) - fmt.Println("OLSH", hotstuffMessage.Round, hotstuffMessage.Step, hotstuffMessage.Type) - return hotstuffMessage.Type == msgType && hotstuffMessage.Step == step } @@ -289,7 +287,6 @@ loop: break loop } case <-ctx.Done(): - fmt.Println("numRemainingMsgs", numRemainingMsgs) if numRemainingMsgs == 0 { break loop } else if numRemainingMsgs > 0 { diff --git a/consensus/pacemaker.go b/consensus/pacemaker.go index 55e685bf1..286c56ef0 100644 --- a/consensus/pacemaker.go +++ b/consensus/pacemaker.go @@ -124,14 +124,14 @@ func (p *paceMaker) ShouldHandleMessage(msg *typesCons.HotstuffMessage) (bool, e // Consensus message is from the past if msg.Height < currentHeight { - p.consensusMod.nodeLog(fmt.Sprintf("[WARN][DISCARDING] Node at height %d received message at a %d", currentHeight, msg.Height)) + p.consensusMod.nodeLog(fmt.Sprintf("⚠️ [WARN][DISCARDING] ⚠️ Node at height %d > message height %d", currentHeight, msg.Height)) return false, nil } // TODO: Need to restart state sync or be in state sync mode right now // Current node is out of sync if msg.Height > currentHeight { - p.consensusMod.nodeLog(fmt.Sprintf("[WARN][DISCARDING] Node at height %d received message at a %d", currentHeight, msg.Height)) + p.consensusMod.nodeLog(fmt.Sprintf("⚠️ [WARN][DISCARDING] ⚠️ Node at height %d < message at height %d", currentHeight, msg.Height)) return false, nil } @@ -147,7 +147,7 @@ func (p *paceMaker) ShouldHandleMessage(msg *typesCons.HotstuffMessage) (bool, e // Message is from the past if msg.Round < currentRound || (msg.Round == currentRound && msg.Step < currentStep) { - p.consensusMod.nodeLog(fmt.Sprintf("[WARN][DISCARDING] Node at (height, step, round) (%d, %d, %d) received message at (%d, %d, %d)", currentHeight, currentStep, currentRound, msg.Height, msg.Step, msg.Round)) + p.consensusMod.nodeLog(fmt.Sprintf("⚠️ [WARN][DISCARDING] ⚠️ Node at (height, step, round) (%d, %d, %d) > message at (%d, %d, %d)", currentHeight, currentStep, currentRound, msg.Height, msg.Step, msg.Round)) return false, nil } From 1aeaca4c08f213e97ada595c0cbbbd733a613b7d Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 2 Jan 2023 14:12:02 -0500 Subject: [PATCH 10/21] Improved TestHotstuff4Nodes1BlockHappyPath --- Makefile | 4 +- consensus/e2e_tests/hotstuff_test.go | 92 +++++++++---------- consensus/e2e_tests/pacemaker_test.go | 92 ++++++++++--------- consensus/e2e_tests/utils_test.go | 7 +- .../sortition/sortition_test.go | 2 +- utility/test/block_test.go | 4 +- 6 files changed, 104 insertions(+), 97 deletions(-) diff --git a/Makefile b/Makefile index 54f462f77..e061cd554 100644 --- a/Makefile +++ b/Makefile @@ -308,9 +308,9 @@ test_consensus_e2e: ## Run all go t2e unit tests in the consensus module w/ log .PHONY: test_consensus_concurrent_tests test_consensus_concurrent_tests: ## Run unit tests in the consensus module that could be prone to race conditions (#192) - for i in $$(seq 1 100); do go test -timeout 2s -count=1 -run ^TestTinyPacemakerTimeouts$ ./consensus/e2e_tests; done; + for i in $$(seq 1 100); do go test -timeout 2s -count=1 -run ^TestPacemakerTimeoutIncreasesRound$ ./consensus/e2e_tests; done; for i in $$(seq 1 100); do go test -timeout 2s -count=1 -run ^TestHotstuff4Nodes1BlockHappyPath$ ./consensus/e2e_tests; done; - for i in $$(seq 1 100); do go test -timeout 2s -count=1 -race -run ^TestTinyPacemakerTimeouts$ ./consensus/e2e_tests; done; + for i in $$(seq 1 100); do go test -timeout 2s -count=1 -race -run ^TestPacemakerTimeoutIncreasesRound$ ./consensus/e2e_tests; done; for i in $$(seq 1 100); do go test -timeout 2s -count=1 -race -run ^TestHotstuff4Nodes1BlockHappyPath$ ./consensus/e2e_tests; done; .PHONY: test_hotstuff diff --git a/consensus/e2e_tests/hotstuff_test.go b/consensus/e2e_tests/hotstuff_test.go index 3ec9036e7..e35bfd90c 100644 --- a/consensus/e2e_tests/hotstuff_test.go +++ b/consensus/e2e_tests/hotstuff_test.go @@ -13,26 +13,24 @@ import ( ) func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { + // Test preparation clockMock := clock.NewMock() - // Test configs - runtimeMgrs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) - - go timeReminder(t, clockMock, time.Second) + timeReminder(t, clockMock, time.Second) // Create & start test pocket nodes - testChannel := make(modules.EventsChannel, 100) - pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, testChannel) + eventsChannel := make(modules.EventsChannel, 100) + runtimeMgrs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) + pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, eventsChannel) StartAllTestPocketNodes(t, pocketNodes) // Debug message to start consensus by triggering first view change for _, pocketNode := range pocketNodes { TriggerNextView(t, pocketNode) } - advanceTime(t, clockMock, 10*time.Millisecond) - // NewRound - newRoundMessages, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 1000, false) + // 1. NewRound + newRoundMessages, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators*numValidators, 250, true) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -45,19 +43,19 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { nodeState) require.Equal(t, false, nodeState.IsLeader) } + for _, message := range newRoundMessages { P2PBroadcast(t, pocketNodes, message) } + advanceTime(t, clockMock, 10*time.Millisecond) + // IMPROVE: Use seeding for deterministic leader election in unit tests. // Leader election is deterministic for now, so we know its NodeId - // TODO(olshansky): Use seeding for deterministic leader election in unit tests. leaderId := typesCons.NodeId(2) leader := pocketNodes[leaderId] - advanceTime(t, clockMock, 10*time.Millisecond) - - // Prepare - prepareProposal, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Prepare, consensus.Propose, 1, 1000, false) + // 2. Prepare + prepareProposal, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Prepare, consensus.Propose, numValidators, 250, true) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -70,22 +68,22 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { nodeState) require.Equal(t, leaderId, nodeState.LeaderId, fmt.Sprintf("%d should be the current leader", leaderId)) } + for _, message := range prepareProposal { P2PBroadcast(t, pocketNodes, message) } - advanceTime(t, clockMock, 10*time.Millisecond) - // Precommit - prepareVotes, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Prepare, consensus.Vote, numValidators, 1000, false) + // 3. PreCommit + prepareVotes, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Prepare, consensus.Vote, numValidators, 250, true) require.NoError(t, err) + for _, vote := range prepareVotes { P2PSend(t, leader, vote) } - advanceTime(t, clockMock, 10*time.Millisecond) - preCommitProposal, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.PreCommit, consensus.Propose, 1, 1000, false) + preCommitProposal, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.PreCommit, consensus.Propose, numValidators, 250, true) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -98,22 +96,22 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { nodeState) require.Equal(t, leaderId, nodeState.LeaderId, fmt.Sprintf("%d should be the current leader", leaderId)) } + for _, message := range preCommitProposal { P2PBroadcast(t, pocketNodes, message) } - advanceTime(t, clockMock, 10*time.Millisecond) - // Commit - preCommitVotes, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.PreCommit, consensus.Vote, numValidators, 1000, false) + // 4. Commit + preCommitVotes, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.PreCommit, consensus.Vote, numValidators, 250, true) require.NoError(t, err) + for _, vote := range preCommitVotes { P2PSend(t, leader, vote) } - advanceTime(t, clockMock, 10*time.Millisecond) - commitProposal, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Commit, consensus.Propose, 1, 1000, false) + commitProposal, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Commit, consensus.Propose, numValidators, 250, true) require.NoError(t, err) for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -126,22 +124,22 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { nodeState) require.Equal(t, leaderId, nodeState.LeaderId, fmt.Sprintf("%d should be the current leader", leaderId)) } + for _, message := range commitProposal { P2PBroadcast(t, pocketNodes, message) } - advanceTime(t, clockMock, 10*time.Millisecond) - // Decide - commitVotes, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Commit, consensus.Vote, numValidators, 1000, false) + // 5. Decide + commitVotes, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Commit, consensus.Vote, numValidators, 250, true) require.NoError(t, err) + for _, vote := range commitVotes { P2PSend(t, leader, vote) } - advanceTime(t, clockMock, 10*time.Millisecond) - decideProposal, err := WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.Decide, consensus.Propose, 1, 1000, false) + decideProposal, err := WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Decide, consensus.Propose, numValidators, 250, true) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -166,14 +164,14 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { nodeState) require.Equal(t, leaderId, nodeState.LeaderId, fmt.Sprintf("%d should be the current leader", leaderId)) } + for _, message := range decideProposal { P2PBroadcast(t, pocketNodes, message) } - advanceTime(t, clockMock, 10*time.Millisecond) - // Block has been committed and new round has begun - _, err = WaitForNetworkConsensusEvents(t, clockMock, testChannel, consensus.NewRound, consensus.Propose, numValidators, 1000, false) + // 1. NewRound - begin again + _, err = WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.NewRound, consensus.Propose, numValidators*numValidators, 250, true) require.NoError(t, err) for pocketId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) @@ -188,55 +186,55 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { } } -/* +// TODO: Implement these tests and use them as a starting point for new ones. Consider using ChatGPT to help you out :) + func TestHotstuff4Nodes1Byzantine1Block(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4Nodes2Byzantine1Block(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4Nodes1BlockNetworkPartition(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4Nodes1Block4Rounds(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4Nodes2Blocks(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4Nodes2NewNodes1Block(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4Nodes2DroppedNodes1Block(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4NodesFailOnPrepare(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4NodesFailOnPrecommit(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4NodesFailOnCommit(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuff4NodesFailOnDecide(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuffValidatorWithLockedQC(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestHotstuffValidatorWithLockedQCMissingNewRoundMsg(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } -*/ diff --git a/consensus/e2e_tests/pacemaker_test.go b/consensus/e2e_tests/pacemaker_test.go index 7f340b65c..036b71c3a 100644 --- a/consensus/e2e_tests/pacemaker_test.go +++ b/consensus/e2e_tests/pacemaker_test.go @@ -15,7 +15,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" ) -func TestTinyPacemakerTimeouts(t *testing.T) { +func TestPacemakerTimeoutIncreasesRound(t *testing.T) { // Test preparation clockMock := clock.NewMock() timeReminder(t, clockMock, time.Second) @@ -23,9 +23,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { // UnitTestNet configs paceMakerTimeoutMsec := uint64(500) // Set a small pacemaker timeout paceMakerTimeout := time.Duration(paceMakerTimeoutMsec) * time.Millisecond - // Must be smaller than pacemaker timeout for the test to work properly because we are waiting - // for a deterministic number of consensus messages. - consensusMessageTimeoutMsec := time.Duration(paceMakerTimeoutMsec / 5) + consensusMessageTimeoutMsec := time.Duration(paceMakerTimeoutMsec / 5) // Must be smaller than pacemaker timeout because we expect a deterministic number of consensus messages. runtimeMgrs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) for _, runtimeConfig := range runtimeMgrs { if consCfg, ok := runtimeConfig.GetConfig().GetConsensusConfig().(consensus.HasPacemakerConfig); ok { @@ -60,7 +58,7 @@ func TestTinyPacemakerTimeouts(t *testing.T) { GetConsensusNodeState(pocketNode)) } - // // Force the pacemaker to time out + // Force the pacemaker to time out forcePacemakerTimeout(t, clockMock, paceMakerTimeout) // Verify that a new round started at the same height @@ -129,25 +127,34 @@ func TestTinyPacemakerTimeouts(t *testing.T) { } func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { + // Test preparation clockMock := clock.NewMock() - runtimeConfigs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) - timeReminder(t, clockMock, time.Second) - // Create & start test pocket nodes - eventsChannel := make(modules.EventsChannel, 100) - pocketNodes := CreateTestConsensusPocketNodes(t, runtimeConfigs, eventsChannel) - StartAllTestPocketNodes(t, pocketNodes) - // Starting point testHeight := uint64(3) testStep := consensus.NewRound - // Leader info - leaderId := typesCons.NodeId(3) // TODO(olshansky): Same as height % numValidators until we add back leader election - leader := pocketNodes[leaderId] - leaderRound := uint64(6) + // UnitTestNet configs + paceMakerTimeoutMsec := uint64(500) // Set a small pacemaker timeout + // paceMakerTimeout := time.Duration(paceMakerTimeoutMsec) * time.Millisecond + runtimeMgrs := GenerateNodeRuntimeMgrs(t, numValidators, clockMock) + for _, runtimeConfig := range runtimeMgrs { + if consCfg, ok := runtimeConfig.GetConfig().GetConsensusConfig().(consensus.HasPacemakerConfig); ok { + consCfg.GetPacemakerConfig().SetTimeoutMsec(paceMakerTimeoutMsec) + } + } + // Create & start test pocket nodes + eventsChannel := make(modules.EventsChannel, 100) + pocketNodes := CreateTestConsensusPocketNodes(t, runtimeMgrs, eventsChannel) + StartAllTestPocketNodes(t, pocketNodes) + + // Prepare leader info + leaderId := typesCons.NodeId(3) + require.Equal(t, uint64(leaderId), testHeight%numValidators) // Uses our deterministic round robin leader election + leaderRound := uint64(6) + leader := pocketNodes[leaderId] consensusPK, err := leader.GetBus().GetConsensusModule().GetPrivateKey() require.NoError(t, err) @@ -170,7 +177,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { // Set all nodes to the same STEP and HEIGHT BUT different ROUNDS for _, pocketNode := range pocketNodes { - //update height, step, leaderid, and utility context via setters exposed with the debug interface + // Update height, step, leaderId, and utility context via setters exposed with the debug interface consensusModImpl := GetConsensusModImpl(pocketNode) consensusModImpl.MethodByName("SetHeight").Call([]reflect.Value{reflect.ValueOf(testHeight)}) consensusModImpl.MethodByName("SetStep").Call([]reflect.Value{reflect.ValueOf(testStep)}) @@ -201,13 +208,12 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { P2PBroadcast(t, pocketNodes, anyMsg) - // numNodes-1 because one of the messages is a self-proposal that is not passed through the network - _, err = WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Prepare, consensus.Vote, numValidators-1, 2000, false) + numExpectedMsgs := numValidators - 1 // -1 because one of the messages is a self proposal (leader to itself as a replica) that is not passed through the network + msgTimeout := paceMakerTimeoutMsec / 2 // /2 because we do not want the pacemaker to trigger a new timeout + _, err = WaitForNetworkConsensusEvents(t, clockMock, eventsChannel, consensus.Prepare, consensus.Vote, numExpectedMsgs, time.Duration(msgTimeout), true) require.NoError(t, err) - forcePacemakerTimeout(t, clockMock, 600*time.Millisecond) - - // Check that the leader is in the latest round. + // Check that all the nodes caught up to the leader's (i.e. the latest) round for nodeId, pocketNode := range pocketNodes { nodeState := GetConsensusNodeState(pocketNode) if nodeId == leaderId { @@ -221,46 +227,46 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { } } -/* +func forcePacemakerTimeout(t *testing.T, clockMock *clock.Mock, paceMakerTimeout timePkg.Duration) { + go func() { + // Cause the pacemaker to timeout + sleep(t, clockMock, paceMakerTimeout) + }() + runtime.Gosched() + // advance time by an amount longer than the timeout + advanceTime(t, clockMock, paceMakerTimeout+10*time.Millisecond) +} + +// TODO: Implement these tests and use them as a starting point for new ones. Consider using ChatGPT to help you out :) + func TestPacemakerDifferentHeightsCatchup(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestPacemakerDifferentStepsCatchup(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } -func TestPacemakerDifferentRoudnsCatchup(t *testing.T) { - t.Skip() // TODO: Implement +func TestPacemakerDifferentRoundsCatchup(t *testing.T) { + t.Skip() } func TestPacemakerWithLockedQC(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestPacemakerWithHighPrepareQC(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestPacemakerNoQuorum(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestPacemakerNotSafeProposal(t *testing.T) { - t.Skip() // TODO: Implement + t.Skip() } func TestPacemakerExponentialTimeouts(t *testing.T) { - t.Skip() // TODO: Implement -} -*/ - -func forcePacemakerTimeout(t *testing.T, clockMock *clock.Mock, paceMakerTimeout timePkg.Duration) { - go func() { - // Cause the pacemaker to timeout - sleep(t, clockMock, paceMakerTimeout) - }() - runtime.Gosched() - // advance time by an amount longer than the timeout - advanceTime(t, clockMock, paceMakerTimeout+10*time.Millisecond) + t.Skip() } diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index bf72a5c7a..beb7acbd4 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -286,13 +286,16 @@ loop: if numRemainingMsgs == 0 && !failOnExtraMessages { break loop } + // The reason we return we format and return an error message rather than using t.Fail(...) + // is to allow the called to run `require.NoError(t, err)` and have the output point to the + // specific line where the test failed. case <-ctx.Done(): if numRemainingMsgs == 0 { break loop } else if numRemainingMsgs > 0 { - t.Fatalf("Missing '%s' messages; %d expected but %d received. (%s)", eventContentType, numExpectedMsgs, len(expectedMsgs), errMsg) + return expectedMsgs, fmt.Errorf("Missing '%s' messages; %d expected but %d received. (%s)", eventContentType, numExpectedMsgs, len(expectedMsgs), errMsg) } else { - t.Fatalf("Too many '%s' messages; %d expected but %d received. (%s)", eventContentType, numExpectedMsgs, len(expectedMsgs), errMsg) + return expectedMsgs, fmt.Errorf("Too many '%s' messages; %d expected but %d received. (%s)", eventContentType, numExpectedMsgs, len(expectedMsgs), errMsg) } } } diff --git a/consensus/leader_election/sortition/sortition_test.go b/consensus/leader_election/sortition/sortition_test.go index e6aaf05d3..8ba0938b5 100644 --- a/consensus/leader_election/sortition/sortition_test.go +++ b/consensus/leader_election/sortition/sortition_test.go @@ -21,7 +21,7 @@ const ( // the hood. func TestSortition(t *testing.T) { // The number of validators in the network - numValidators := uint64(1000) // // NOTE: This value is somewhat arbitrary. + numValidators := uint64(1000) // NOTE: This value is somewhat arbitrary. uPOKTNetworkStake := uPOKTMinValidatorStake * numValidators testParameters := []struct { diff --git a/utility/test/block_test.go b/utility/test/block_test.go index a0394f56a..9aa6ea5cf 100644 --- a/utility/test/block_test.go +++ b/utility/test/block_test.go @@ -32,7 +32,7 @@ func TestUtilityContext_ApplyBlock(t *testing.T) { require.NoError(t, err) require.NotNil(t, appHash) - // // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. + // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. // beginBlock logic verify // missed, err := ctx.GetValidatorMissedBlocks(byzantine.Address) // require.NoError(t, err) @@ -82,7 +82,7 @@ func TestUtilityContext_BeginBlock(t *testing.T) { _, er = ctx.ApplyBlock() require.NoError(t, er) - // // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. + // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. // beginBlock logic verify // missed, err := ctx.GetValidatorMissedBlocks(byzantine.Address) // require.NoError(t, err) From b1cdf48de4d1835d156ce49339f95c46eae56c40 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 2 Jan 2023 14:39:14 -0500 Subject: [PATCH 11/21] Self review - round 1 --- consensus/doc/CHANGELOG.md | 2 + consensus/e2e_tests/block_proposal_test.go | 104 --------------------- consensus/e2e_tests/utils_test.go | 6 +- consensus/helpers.go | 3 +- 4 files changed, 8 insertions(+), 107 deletions(-) delete mode 100644 consensus/e2e_tests/block_proposal_test.go diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index 25925d73f..40291599b 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.14] - 2023-01-03 + ## [0.0.0.13] - 2022-12-14 - Consolidated number of validators in tests in a single constant: `numValidators` diff --git a/consensus/e2e_tests/block_proposal_test.go b/consensus/e2e_tests/block_proposal_test.go deleted file mode 100644 index 15e7e38e3..000000000 --- a/consensus/e2e_tests/block_proposal_test.go +++ /dev/null @@ -1,104 +0,0 @@ -package e2e_tests - -// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node4.consensus | [00] [NODE][4] [DEBUG] Triggering next view... -// node4.consensus | [00] [NODE][4] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 0)! Reason: manual trigger -// node4.consensus | [00] [NODE][4] Broadcasting message for HOTSTUFF_STEP_NEWROUND step -// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node3.consensus | [00] [NODE][3] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node1.consensus | [00] [NODE][1] [DEBUG] Triggering next view... -// node1.consensus | [00] [NODE][1] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 0)! Reason: manual trigger -// node1.consensus | [00] [NODE][1] Broadcasting message for HOTSTUFF_STEP_NEWROUND step -// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node3.consensus | [00] [NODE][3] pacemaker catching up the node's (height, step, round) FROM (12, HOTSTUFF_STEP_NEWROUND, 0) TO (12, HOTSTUFF_STEP_NEWROUND, 1) -// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node4.consensus | [00] [NODE][4] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node1.consensus | [00] [NODE][1] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node2.consensus | [00] [NODE][2] [DEBUG] Triggering next view... -// node2.consensus | [00] [NODE][2] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 0)! Reason: manual trigger -// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node2.consensus | [00] [NODE][2] Broadcasting message for HOTSTUFF_STEP_NEWROUND step -// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node2.consensus | [00] [NODE][2] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node4.consensus | [00] [REPLICA][4] 🙇🙇🙇 Elected leader for height/round 12/1: [2] (3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99) 🙇🙇🙇 -// node4.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_REPLICA -// node3.consensus | [00] [REPLICA][3] 🙇🙇🙇 Elected leader for height/round 12/1: [2] (3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99) 🙇🙇🙇 -// node3.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_REPLICA -// node1.consensus | [00] [REPLICA][1] 🙇🙇🙇 Elected leader for height/round 12/1: [2] (3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99) 🙇🙇🙇 -// node2.consensus | [00] [LEADER][2] 👑👑👑 I am the leader for height/round 12/1: [2] (3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99) 👑👑👑 -// node1.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_REPLICA -// node2.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER -// node3.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER -// node4.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node2.consensus | [00] [LEADER][2] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (1 > 2.67?) -// node2.consensus | [00] [LEADER][2] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node1.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node3.consensus | [00] [REPLICA][3] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (1 > 2.67?) -// node4.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER -// node2.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER -// node2.consensus | [00] [LEADER][2] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (2 > 2.67?) -// node1.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 1 VALIDATOR_TYPE_LEADER -// node3.consensus | [00] [REPLICA][3] [DEBUG] Triggering next view... -// node4.consensus | [00] [REPLICA][4] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (1 > 2.67?) -// node2.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric height 12 -// node1.consensus | [00] [REPLICA][1] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (1 > 2.67?) -// node1.consensus | [00] [REPLICA][1] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node1.consensus | [00] [REPLICA][1] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 2. -// node1.consensus | [00] [REPLICA][1] pacemaker catching up the node's (height, step, round) FROM (12, HOTSTUFF_STEP_PREPARE, 1) TO (12, HOTSTUFF_STEP_NEWROUND, 2) -// node4.consensus | [00] [REPLICA][4] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node3.consensus | [00] [REPLICA][3] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_PREPARE, 1)! Reason: manual trigger -// node1.consensus | [00] [REPLICA][1] 🙇🙇🙇 Elected leader for height/round 12/2: [3] (67eb3f0a50ae459fecf666be0e93176e92441317) 🙇🙇🙇 -// node2.consensus | [00] [LEADER][2] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 2. -// node4.consensus | [00] [REPLICA][4] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 2. -// node3.consensus | [00] About to release postgres context at height 12. -// node2.consensus | [00] [LEADER][2] pacemaker catching up the node's (height, step, round) FROM (12, HOTSTUFF_STEP_NEWROUND, 1) TO (12, HOTSTUFF_STEP_NEWROUND, 2) -// node2.consensus | [00] [REPLICA][2] 🙇🙇🙇 Elected leader for height/round 12/2: [3] (67eb3f0a50ae459fecf666be0e93176e92441317) 🙇🙇🙇 -// node3.consensus | [00] [NODE][3] Broadcasting message for HOTSTUFF_STEP_NEWROUND step -// node4.consensus | [00] [REPLICA][4] pacemaker catching up the node's (height, step, round) FROM (12, HOTSTUFF_STEP_PREPARE, 1) TO (12, HOTSTUFF_STEP_NEWROUND, 2) -// node1.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_REPLICA -// node1.consensus | [00] [REPLICA][1] [WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it -// node1.consensus | [00] About to release postgres context at height 12. -// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node2.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_REPLICA -// node1.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_LEADER -// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node4.consensus | [00] [REPLICA][4] 🙇🙇🙇 Elected leader for height/round 12/2: [3] (67eb3f0a50ae459fecf666be0e93176e92441317) 🙇🙇🙇 -// node4.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_REPLICA -// node4.consensus | [00] [REPLICA][4] [WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it -// node1.consensus | [00] [REPLICA][1] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (2 > 2.67?) -// node3.consensus | [00] [EVENT] event_metrics_namespace_p2p raintree_message_event_metric send send -// node4.consensus | [00] About to release postgres context at height 12. -// node2.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_LEADER -// node3.consensus | [00] [NODE][3] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node3.consensus | [00] [NODE][3] [DEBUG] Handling message w/ Height: 12; Type: HOTSTUFF_STEP_NEWROUND; Round: 1. -// node4.consensus | [00] [EVENT] event_metrics_namespace_consensus hotpokt_message_event_metric HEIGHT 12 HOTSTUFF_STEP_NEWROUND 2 VALIDATOR_TYPE_LEADER -// node2.consensus | [00] [REPLICA][2] received enough HOTSTUFF_STEP_NEWROUND votes! -// node4.consensus | [00] [REPLICA][4] Waiting for more HOTSTUFF_STEP_NEWROUND messages; byzantine optimistic threshold not met: (2 > 2.67?) -// node2.consensus | [00] [REPLICA][2] [WARN] utilityContext expected to be nil but is not. TODO: Investigate why this is and fix it -// node2.consensus | [00] About to release postgres context at height 12. -// node2.consensus | [00] [REPLICA][2] Preparing a new block - no highPrepareQC found -// node2.consensus | [00] [ERROR][REPLICA][2] could not prepare block: node should not call `prepareBlock` if it is not a leader -// node2.consensus | [00] [REPLICA][2] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_PREPARE, 2)! Reason: failed to prepare & apply block -// node2.consensus | [00] About to release postgres context at height 12. -// node3.consensus | [00] [NODE][3] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 2)! Reason: timeout -// node4.consensus | [00] [REPLICA][4] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_PREPARE, 2)! Reason: timeout -// node4.consensus | [00] About to release postgres context at height 12. -// node1.consensus | [00] [REPLICA][1] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_PREPARE, 2)! Reason: timeout -// node1.consensus | [00] About to release postgres context at height 12. -// node2.consensus | [00] [NODE][2] INTERRUPT at (height, step, round): (12, HOTSTUFF_STEP_NEWROUND, 3)! Reason: timeout diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index beb7acbd4..9cbd0476a 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -261,6 +261,10 @@ func waitForEventsInternal( } } }() + defer ticker.Stop() + defer func() { + tickerDone <- true + }() numRemainingMsgs := numExpectedMsgs loop: @@ -299,8 +303,6 @@ loop: } } } - ticker.Stop() - tickerDone <- true for _, u := range unusedEvents { eventsChannel <- u diff --git a/consensus/helpers.go b/consensus/helpers.go index 4816d1be3..64a93ba32 100644 --- a/consensus/helpers.go +++ b/consensus/helpers.go @@ -158,7 +158,8 @@ func (m *consensusModule) sendToLeader(msg *typesCons.HotstuffMessage) { } } -// Star-like "broadcast" - send to all nodes directly +// Star-like (O(n)) broadcast - send to all nodes directly +// INVESTIGATE: Re-evaluate if we should be using our structured broadcast (RainTree O(log3(n))) algorithm instead func (m *consensusModule) broadcastToValidators(msg *typesCons.HotstuffMessage) { m.nodeLog(typesCons.BroadcastingMessage(msg)) From fe1aa3992f777856fd2b6cf7f20e54d6bf4ab6f3 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 2 Jan 2023 14:45:16 -0500 Subject: [PATCH 12/21] Update changelog --- consensus/doc/CHANGELOG.md | 19 +++++++++++++++++++ p2p/CHANGELOG.md | 4 ++++ shared/CHANGELOG.md | 4 ++++ 3 files changed, 27 insertions(+) diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index 40291599b..49516466f 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -9,6 +9,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.0.0.14] - 2023-01-03 +### Consensus - Core + +- Force consensus to use a "star-like" broadcast instead of "RainTree" broadcast +- Improve logging throughout through the use of emojis and rewording certain statements +- Slightly improve the block verification flow (renaming, minor fixes, etc…) to stabilize LocalNet + +### Consensus - Tests + +- Rename the `consensus_tests` package to `e2e_tests` +- Internalize configuration related to `fail_on_extra_msgs` from the `Makefile` to the `consensus` module +- Forced all tests to fail if we receive extra unexpected messages and modify tests appropriately +- After #198, we made tests deterministic but there was a hidden bug that modified how the test utility functions because the clock would not move while we were waiting for messages. This prevented logs from streaming, tests from failing, and other issues. Tend to all related changes. + +### Consensus - Pacemaker + +- Rename `ValidateMessage` to `ShouldHandleMessage` and return a boolean +- Pass a `reason` to `InterruptRoudn` +- Improve readability of some parts of the code + ## [0.0.0.13] - 2022-12-14 - Consolidated number of validators in tests in a single constant: `numValidators` diff --git a/p2p/CHANGELOG.md b/p2p/CHANGELOG.md index a54dfd530..58a5bea13 100644 --- a/p2p/CHANGELOG.md +++ b/p2p/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.13] - 2023-01-03 + +- Add a lock to the mempool to avoid parallel messages which has caused the node to crash in the past + ## [0.0.0.12] - 2022-12-16 - `ValidatorMapToAddrBook` renamed to `ActorToAddrBook` diff --git a/shared/CHANGELOG.md b/shared/CHANGELOG.md index 657e3ccc1..d96afff95 100644 --- a/shared/CHANGELOG.md +++ b/shared/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.7] - 2023-01-03 + +- Make the events channel hold pointers rather than copies of the message + ## [0.0.0.6] - 2022-12-14 - Added `GetMaxMempoolCount` From f24ce1063e8577a97dba491dcf270ba6c4cba30b Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 9 Jan 2023 14:08:07 -0500 Subject: [PATCH 13/21] Addressing review comments --- build/config/genesis.json | 280 ++---------------------- consensus/block.go | 19 +- consensus/doc/CHANGELOG.md | 2 +- consensus/e2e_tests/pacemaker_test.go | 4 +- consensus/e2e_tests/utils_test.go | 4 +- consensus/hotstuff_leader.go | 2 +- consensus/hotstuff_replica.go | 2 +- consensus/pacemaker.go | 9 +- docs/demos/iteration_3_end_to_end_tx.md | 16 +- runtime/manager_test.go | 31 ++- shared/CHANGELOG.md | 2 +- 11 files changed, 63 insertions(+), 308 deletions(-) diff --git a/build/config/genesis.json b/build/config/genesis.json index 322e53e87..003c30393 100755 --- a/build/config/genesis.json +++ b/build/config/genesis.json @@ -1,256 +1,8 @@ { -<<<<<<< HEAD - "persistence_genesis_state": { - "accounts": [ - { - "address": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", - "amount": "100000000000000" - }, - { - "address": "67eb3f0a50ae459fecf666be0e93176e92441317", - "amount": "100000000000000" - }, - { - "address": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", - "amount": "100000000000000" - }, - { - "address": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", - "amount": "100000000000000" - }, - { - "address": "43d9ea9d9ad9c58bb96ec41340f83cb2cabb6496", - "amount": "100000000000000" - }, - { - "address": "9ba047197ec043665ad3f81278ab1f5d3eaf6b8b", - "amount": "100000000000000" - }, - { - "address": "88a792b7aca673620132ef01f50e62caa58eca83", - "amount": "100000000000000" - } - ], - "pools": [ - { - "address": "DAO", - "amount": "100000000000000" - }, - { - "address": "FeeCollector", - "amount": "0" - }, - { - "address": "AppStakePool", - "amount": "100000000000000" - }, - { - "address": "ValidatorStakePool", - "amount": "100000000000000" - }, - { - "address": "ServiceNodeStakePool", - "amount": "100000000000000" - }, - { - "address": "FishermanStakePool", - "amount": "100000000000000" - } - ], - "validators": [ - { - "address": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", - "public_key": "53ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", - "chains": null, - "generic_param": "node1.consensus:8080", - "staked_amount": "1000000000000", - "paused_height": -1, - "unstaking_height": -1, - "output": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", - "actor_type": 3 - }, - { - "address": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", - "public_key": "a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2", - "chains": null, - "generic_param": "node2.consensus:8080", - "staked_amount": "1000000000000", - "paused_height": -1, - "unstaking_height": -1, - "output": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", - "actor_type": 3 - }, - { - "address": "67eb3f0a50ae459fecf666be0e93176e92441317", - "public_key": "c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb", - "chains": null, - "generic_param": "node3.consensus:8080", - "staked_amount": "1000000000000", - "paused_height": -1, - "unstaking_height": -1, - "output": "67eb3f0a50ae459fecf666be0e93176e92441317", - "actor_type": 3 - }, - { - "address": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", - "public_key": "b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", - "chains": null, - "generic_param": "node4.consensus:8080", - "staked_amount": "1000000000000", - "paused_height": -1, - "unstaking_height": -1, - "output": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", - "actor_type": 3 - } - ], - "applications": [], - "service_nodes": [ - { - "address": "43d9ea9d9ad9c58bb96ec41340f83cb2cabb6496", - "public_key": "16cd0a304c38d76271f74dd3c90325144425d904ef1b9a6fbab9b201d75a998b", - "chains": ["0001"], - "generic_param": "node1.consensus:8080", - "staked_amount": "1000000000000", - "paused_height": -1, - "unstaking_height": -1, - "output": "43d9ea9d9ad9c58bb96ec41340f83cb2cabb6496", - "actor_type": 1 - } - ], - "fishermen": [ - { - "address": "9ba047197ec043665ad3f81278ab1f5d3eaf6b8b", - "public_key": "68efd26af01692fcd77dc135ca1de69ede464e8243e6832bd6c37f282db8c9cb", - "chains": ["0001"], - "generic_param": "node1.consensus:8080", - "staked_amount": "1000000000000", - "paused_height": -1, - "unstaking_height": -1, - "output": "9ba047197ec043665ad3f81278ab1f5d3eaf6b8b", - "actor_type": 2 - } - ], - "params": { - "blocks_per_session": 4, - "app_minimum_stake": "15000000000", - "app_max_chains": 15, - "app_baseline_stake_rate": 100, - "app_unstaking_blocks": 2016, - "app_minimum_pause_blocks": 4, - "app_max_pause_blocks": 672, - "service_node_minimum_stake": "15000000000", - "service_node_max_chains": 15, - "service_node_unstaking_blocks": 2016, - "service_node_minimum_pause_blocks": 4, - "service_node_max_pause_blocks": 672, - "service_nodes_per_session": 24, - "fisherman_minimum_stake": "15000000000", - "fisherman_max_chains": 15, - "fisherman_unstaking_blocks": 2016, - "fisherman_minimum_pause_blocks": 4, - "fisherman_max_pause_blocks": 672, - "validator_minimum_stake": "15000000000", - "validator_unstaking_blocks": 2016, - "validator_minimum_pause_blocks": 4, - "validator_max_pause_blocks": 672, - "validator_maximum_missed_blocks": 5, - "validator_max_evidence_age_in_blocks": 8, - "proposer_percentage_of_fees": 10, - "missed_blocks_burn_percentage": 1, - "double_sign_burn_percentage": 5, - "message_double_sign_fee": "10000", - "message_send_fee": "10000", - "message_stake_fisherman_fee": "10000", - "message_edit_stake_fisherman_fee": "10000", - "message_unstake_fisherman_fee": "10000", - "message_pause_fisherman_fee": "10000", - "message_unpause_fisherman_fee": "10000", - "message_fisherman_pause_service_node_fee": "10000", - "message_test_score_fee": "10000", - "message_prove_test_score_fee": "10000", - "message_stake_app_fee": "10000", - "message_edit_stake_app_fee": "10000", - "message_unstake_app_fee": "10000", - "message_pause_app_fee": "10000", - "message_unpause_app_fee": "10000", - "message_stake_validator_fee": "10000", - "message_edit_stake_validator_fee": "10000", - "message_unstake_validator_fee": "10000", - "message_pause_validator_fee": "10000", - "message_unpause_validator_fee": "10000", - "message_stake_service_node_fee": "10000", - "message_edit_stake_service_node_fee": "10000", - "message_unstake_service_node_fee": "10000", - "message_pause_service_node_fee": "10000", - "message_unpause_service_node_fee": "10000", - "message_change_parameter_fee": "10000", - "acl_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "blocks_per_session_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "app_minimum_stake_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "app_max_chains_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "app_baseline_stake_rate_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "app_staking_adjustment_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "app_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "app_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "app_max_paused_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "service_node_minimum_stake_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "service_node_max_chains_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "service_node_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "service_node_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "service_node_max_paused_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "service_nodes_per_session_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "fisherman_minimum_stake_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "fisherman_max_chains_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "fisherman_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "fisherman_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "fisherman_max_paused_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "validator_minimum_stake_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "validator_unstaking_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "validator_minimum_pause_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "validator_max_paused_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "validator_maximum_missed_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "validator_max_evidence_age_in_blocks_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "proposer_percentage_of_fees_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "missed_blocks_burn_percentage_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "double_sign_burn_percentage_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_double_sign_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_send_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_stake_fisherman_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_edit_stake_fisherman_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_unstake_fisherman_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_pause_fisherman_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_unpause_fisherman_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_fisherman_pause_service_node_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_test_score_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_prove_test_score_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_stake_app_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_edit_stake_app_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_unstake_app_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_pause_app_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_unpause_app_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_stake_validator_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_edit_stake_validator_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_unstake_validator_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_pause_validator_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_unpause_validator_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_stake_service_node_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_edit_stake_service_node_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_unstake_service_node_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_pause_service_node_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_unpause_service_node_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45", - "message_change_parameter_fee_owner": "da034209758b78eaea06dd99c07909ab54c99b45" - } - }, - "consensus_genesis_state": { - "genesis_time": { - "seconds": 1663610702, - "nanos": 405401000 -======= "accounts": [ { "address": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", "amount": "100000000000000" ->>>>>>> main }, { "address": "67eb3f0a50ae459fecf666be0e93176e92441317", @@ -305,48 +57,48 @@ ], "validators": [ { - "address": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", - "public_key": "b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", + "address": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", + "public_key": "53ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", "chains": null, "generic_param": "node1.consensus:8080", "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, - "output": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", - "actor_type": 4 + "output": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", + "actor_type": 3 }, { - "address": "67eb3f0a50ae459fecf666be0e93176e92441317", - "public_key": "c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb", + "address": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", + "public_key": "a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2", "chains": null, "generic_param": "node2.consensus:8080", "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, - "output": "67eb3f0a50ae459fecf666be0e93176e92441317", - "actor_type": 4 + "output": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", + "actor_type": 3 }, { - "address": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", - "public_key": "a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2", + "address": "67eb3f0a50ae459fecf666be0e93176e92441317", + "public_key": "c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb", "chains": null, "generic_param": "node3.consensus:8080", "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, - "output": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", - "actor_type": 4 + "output": "67eb3f0a50ae459fecf666be0e93176e92441317", + "actor_type": 3 }, { - "address": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", - "public_key": "53ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", + "address": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", + "public_key": "b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", "chains": null, "generic_param": "node4.consensus:8080", "staked_amount": "1000000000000", "paused_height": -1, "unstaking_height": -1, - "output": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", - "actor_type": 4 + "output": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", + "actor_type": 3 } ], "applications": [ diff --git a/consensus/block.go b/consensus/block.go index dad21738b..2b4278883 100644 --- a/consensus/block.go +++ b/consensus/block.go @@ -25,32 +25,33 @@ func (m *consensusModule) commitBlock(block *typesCons.Block) error { return nil } -// TODO: Add unit tests specific to block validation -// IMPROVE: (olshansky) rename to provide clarity of operation. ValidateBasic() is typically a stateless check not stateful -func (m *consensusModule) validateMessageBlock(msg *typesCons.HotstuffMessage) error { +// ADDTEST: Add unit tests specific to block validation +// IMPROVE: Rename to provide clarity of operation. ValidateBasic() is typically a stateless check not stateful +func (m *consensusModule) isValidMessageBlock(msg *typesCons.HotstuffMessage) (bool, error) { block := msg.GetBlock() step := msg.GetStep() if block == nil { if step != NewRound { - return fmt.Errorf("validateBlockBasic failed - block is nil during step %s", typesCons.StepToString[m.step]) + return false, fmt.Errorf("validateBlockBasic failed - block is nil during step %s", typesCons.StepToString[m.step]) } - return nil + m.nodeLog("[DEBUG] Nil (expected) block is present during NewRound step.") + return true, nil } if block != nil && step == NewRound { - return fmt.Errorf("validateBlockBasic failed - block is not nil during step %s", typesCons.StepToString[m.step]) + return false, fmt.Errorf("validateBlockBasic failed - block is not nil during step %s", typesCons.StepToString[m.step]) } if block != nil && unsafe.Sizeof(*block) > uintptr(m.genesisState.GetMaxBlockBytes()) { - return typesCons.ErrInvalidBlockSize(uint64(unsafe.Sizeof(*block)), m.genesisState.GetMaxBlockBytes()) + return false, typesCons.ErrInvalidBlockSize(uint64(unsafe.Sizeof(*block)), m.genesisState.GetMaxBlockBytes()) } // If the current block being processed (i.e. voted on) by consensus is non nil, we need to make // sure that the data (height, round, step, txs, etc) is the same before we start validating the signatures if m.block != nil { if m.block.BlockHeader.Hash != block.BlockHeader.Hash { - return fmt.Errorf("validateBlockBasic failed - block hash is not the same as the current block being processed by consensus") + return false, fmt.Errorf("validateBlockBasic failed - block hash is not the same as the current block being processed by consensus") } // DISCUSS: The only difference between blocks from one step to another is the QC, so we need @@ -60,7 +61,7 @@ func (m *consensusModule) validateMessageBlock(msg *typesCons.HotstuffMessage) e } } - return nil + return true, nil } // Creates a new Utility context and clears/nullifies any previous contexts if they exist diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index 753b1a3b9..d8943ce0c 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Consensus - Pacemaker - Rename `ValidateMessage` to `ShouldHandleMessage` and return a boolean -- Pass a `reason` to `InterruptRoudn` +- Pass a `reason` to `InterruptRound` - Improve readability of some parts of the code ## [0.0.0.16] - 2023-01-03 diff --git a/consensus/e2e_tests/pacemaker_test.go b/consensus/e2e_tests/pacemaker_test.go index 01a3d4937..c8c77f430 100644 --- a/consensus/e2e_tests/pacemaker_test.go +++ b/consensus/e2e_tests/pacemaker_test.go @@ -5,7 +5,6 @@ import ( "runtime" "testing" "time" - timePkg "time" "github.com/benbjohnson/clock" "github.com/pokt-network/pocket/consensus" @@ -21,6 +20,7 @@ func TestPacemakerTimeoutIncreasesRound(t *testing.T) { timeReminder(t, clockMock, time.Second) // UnitTestNet configs + // IMPROVE(#295): Remove time specific suffixes as outlined by go-staticcheck (ST1011) paceMakerTimeoutMsec := uint64(500) // Set a small pacemaker timeout paceMakerTimeout := time.Duration(paceMakerTimeoutMsec) * time.Millisecond consensusMessageTimeoutMsec := time.Duration(paceMakerTimeoutMsec / 5) // Must be smaller than pacemaker timeout because we expect a deterministic number of consensus messages. @@ -224,7 +224,7 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { } } -func forcePacemakerTimeout(t *testing.T, clockMock *clock.Mock, paceMakerTimeout timePkg.Duration) { +func forcePacemakerTimeout(t *testing.T, clockMock *clock.Mock, paceMakerTimeout time.Duration) { go func() { // Cause the pacemaker to timeout sleep(t, clockMock, paceMakerTimeout) diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index 4f20f5528..0467e1f5d 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -207,7 +207,7 @@ func P2PSend(_ *testing.T, node *shared.Node, any *anypb.Any) { // maximum delay specified in the source code and errors if additional unexpected messages are received. // For example, if the test expects to receive 5 messages within 2 seconds: // false: continue if 5 messages are received in 0.5 seconds -// true: true: wait for another 1.5 seconds after 5 messages are received in 0.5 seconds, and fail if any additional messages are received. +// true: wait for another 1.5 seconds after 5 messages are received in 0.5 seconds, and fail if any additional messages are received. func WaitForNetworkConsensusEvents( t *testing.T, clock *clock.Mock, @@ -239,7 +239,7 @@ func waitForEventsInternal( eventsChannel modules.EventsChannel, eventContentType string, numExpectedMsgs int, - maxWaitTimeMillis time.Duration, + maxWaitTimeMillis time.Duration, // IMPROVE(#295): Remove time specific suffixes as outlined by go-staticcheck (ST1011) msgIncludeFilter func(m *anypb.Any) bool, errMsg string, failOnExtraMessages bool, diff --git a/consensus/hotstuff_leader.go b/consensus/hotstuff_leader.go index 99f2f356c..63ff8c288 100644 --- a/consensus/hotstuff_leader.go +++ b/consensus/hotstuff_leader.go @@ -246,7 +246,7 @@ func (handler *HotstuffLeaderMessageHandler) HandleDecideMessage(m *consensusMod func (handler *HotstuffLeaderMessageHandler) anteHandle(m *consensusModule, msg *typesCons.HotstuffMessage) error { // Basic block metadata validation - if err := m.validateMessageBlock(msg); err != nil { + if valid, err := m.isValidMessageBlock(msg); !valid { return err } diff --git a/consensus/hotstuff_replica.go b/consensus/hotstuff_replica.go index ec4ca9731..5ee03d9ac 100644 --- a/consensus/hotstuff_replica.go +++ b/consensus/hotstuff_replica.go @@ -165,7 +165,7 @@ func (handler *HotstuffReplicaMessageHandler) HandleDecideMessage(m *consensusMo // anteHandle is the handler called on every replica message before specific handler func (handler *HotstuffReplicaMessageHandler) anteHandle(m *consensusModule, msg *typesCons.HotstuffMessage) error { // Basic block metadata validation - if err := m.validateMessageBlock(msg); err != nil { + if valid, err := m.isValidMessageBlock(msg); !valid { return err } diff --git a/consensus/pacemaker.go b/consensus/pacemaker.go index 38b7e84a6..6721dfd57 100644 --- a/consensus/pacemaker.go +++ b/consensus/pacemaker.go @@ -4,7 +4,7 @@ import ( "context" "fmt" "log" - timePkg "time" + "time" consensusTelemetry "github.com/pokt-network/pocket/consensus/telemetry" typesCons "github.com/pokt-network/pocket/consensus/types" @@ -14,6 +14,7 @@ import ( const ( pacemakerModuleName = "pacemaker" + timeoutBuffer = 30 * time.Millisecond // A buffer around the pacemaker timeout to avoid race condition; 30ms was arbitrarily chosen ) type Pacemaker interface { @@ -182,7 +183,7 @@ func (p *paceMaker) RestartTimer() { if ctx.Err() == context.DeadlineExceeded { p.InterruptRound("timeout") } - case <-clock.After(stepTimeout + 30*timePkg.Millisecond): // Adding 30ms to the context timeout to avoid race condition. + case <-clock.After(stepTimeout + timeoutBuffer): return } }() @@ -252,7 +253,7 @@ func (p *paceMaker) startNextView(qc *typesCons.QuorumCertificate, forceNextView } // TODO(olshansky): Increase timeout using exponential backoff. -func (p *paceMaker) getStepTimeout(round uint64) timePkg.Duration { - baseTimeout := timePkg.Duration(int64(timePkg.Millisecond) * int64(p.pacemakerCfg.TimeoutMsec)) +func (p *paceMaker) getStepTimeout(round uint64) time.Duration { + baseTimeout := time.Duration(int64(time.Millisecond) * int64(p.pacemakerCfg.TimeoutMsec)) return baseTimeout } diff --git a/docs/demos/iteration_3_end_to_end_tx.md b/docs/demos/iteration_3_end_to_end_tx.md index 38c5fd8c5..016c06c22 100644 --- a/docs/demos/iteration_3_end_to_end_tx.md +++ b/docs/demos/iteration_3_end_to_end_tx.md @@ -103,19 +103,21 @@ Since our Keybase is under development, we have to manually inject the private k For the following steps, you'll need to use the accounts of the first two validators in the hard-coded development genesis file. Therefore you have some options: -1) You can just: +1. You can just: + ```bash echo '"6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4"' > /tmp/val1.json echo '"5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb"' > /tmp/val2.json ``` -2) You can use `jq` and run these commands: - ```bash - cat ./build/config/config1.json | jq '.private_key' > /tmp/val1.json - cat ./build/config/config2.json | jq '.private_key' > /tmp/val2.json - ``` +2. You can use `jq` and run these commands: + + ```bash + cat ./build/config/config1.json | jq '.private_key' > /tmp/val1.json + cat ./build/config/config2.json | jq '.private_key' > /tmp/val2.json + ``` -3) You can manually copy-paste the private keys from the config files into the `/tmp/val1.json` and `/tmp/val2.json` files. Remember to keep the double quotes around the private keys ("private_key" field in the JSON). +3. You can manually copy-paste the private keys from the config files into the `/tmp/val1.json` and `/tmp/val2.json` files. Remember to keep the double quotes around the private keys ("private_key" field in the JSON). ### First Transaction diff --git a/runtime/manager_test.go b/runtime/manager_test.go index 5997e6f2a..c4df83243 100644 --- a/runtime/manager_test.go +++ b/runtime/manager_test.go @@ -93,50 +93,49 @@ var expectedGenesis = &genesis.GenesisState{ }, }, Validators: []*types.Actor{ - { ActorType: types.ActorType_ACTOR_TYPE_VAL, - Address: "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", - PublicKey: "b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", + Address: "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", + PublicKey: "53ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", Chains: nil, GenericParam: "node1.consensus:8080", StakedAmount: "1000000000000", PausedHeight: -1, UnstakingHeight: -1, - Output: "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", + Output: "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", }, { ActorType: types.ActorType_ACTOR_TYPE_VAL, - Address: "67eb3f0a50ae459fecf666be0e93176e92441317", - PublicKey: "c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb", + Address: "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", + PublicKey: "a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2", Chains: nil, GenericParam: "node2.consensus:8080", StakedAmount: "1000000000000", PausedHeight: -1, UnstakingHeight: -1, - Output: "67eb3f0a50ae459fecf666be0e93176e92441317", + Output: "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", }, { ActorType: types.ActorType_ACTOR_TYPE_VAL, - Address: "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", - PublicKey: "a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2", + Address: "67eb3f0a50ae459fecf666be0e93176e92441317", + PublicKey: "c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb", Chains: nil, GenericParam: "node3.consensus:8080", StakedAmount: "1000000000000", PausedHeight: -1, UnstakingHeight: -1, - Output: "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", + Output: "67eb3f0a50ae459fecf666be0e93176e92441317", }, { ActorType: types.ActorType_ACTOR_TYPE_VAL, - Address: "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", - PublicKey: "53ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", + Address: "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", + PublicKey: "b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", Chains: nil, GenericParam: "node4.consensus:8080", StakedAmount: "1000000000000", PausedHeight: -1, UnstakingHeight: -1, - Output: "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", + Output: "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", }, }, ServiceNodes: []*types.Actor{ @@ -200,9 +199,9 @@ func TestNewManagerFromReaders(t *testing.T) { want: &Manager{ &configs.Config{ RootDirectory: "/go/src/github.com/pocket-network", - PrivateKey: "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", + PrivateKey: "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", Consensus: &configs.ConsensusConfig{ - PrivateKey: "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", + PrivateKey: "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", MaxMempoolBytes: 500000000, PacemakerConfig: &configs.PacemakerConfig{ TimeoutMsec: 5000, @@ -222,7 +221,7 @@ func TestNewManagerFromReaders(t *testing.T) { TreesStoreDir: "/var/trees", }, P2P: &configs.P2PConfig{ - PrivateKey: "6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4", + PrivateKey: "c6c136d010d07d7f5e9944aa3594a10f9210dd3e26ebc1bc1516a6d957fd0df353ee26c82826694ffe1773d7b60d5f20dd9e91bdf8745544711bec5ff9c6fb4a", ConsensusPort: 8080, UseRainTree: true, IsEmptyConnectionType: false, diff --git a/shared/CHANGELOG.md b/shared/CHANGELOG.md index f8863e5db..376d64310 100644 --- a/shared/CHANGELOG.md +++ b/shared/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.9] - 2023-01-03 +## [0.0.0.9] - 2023-01-09 - Make the events channel hold pointers rather than copies of the message From 3d0069874a41aa44d1d51bd6a27e72c89a37bd14 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 9 Jan 2023 14:09:16 -0500 Subject: [PATCH 14/21] Update consensus/e2e_tests/hotstuff_test.go This is to ensure in the beginning all nodes set leader id to 0, to prevent some possible bugs. Co-authored-by: goku <118421317+gokutheengineer@users.noreply.github.com> --- consensus/e2e_tests/hotstuff_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/consensus/e2e_tests/hotstuff_test.go b/consensus/e2e_tests/hotstuff_test.go index e35bfd90c..a1961654b 100644 --- a/consensus/e2e_tests/hotstuff_test.go +++ b/consensus/e2e_tests/hotstuff_test.go @@ -42,6 +42,7 @@ func TestHotstuff4Nodes1BlockHappyPath(t *testing.T) { }, nodeState) require.Equal(t, false, nodeState.IsLeader) + require.Equal(t, typesCons.NodeId(0), nodeState.LeaderId) } for _, message := range newRoundMessages { From a0eb86b7f3e8f96f996c2771675837d6fb648698 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 9 Jan 2023 15:30:09 -0500 Subject: [PATCH 15/21] Fix actor tyoe --- build/config/genesis.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/build/config/genesis.json b/build/config/genesis.json index 003c30393..3760715d4 100755 --- a/build/config/genesis.json +++ b/build/config/genesis.json @@ -65,7 +65,7 @@ "paused_height": -1, "unstaking_height": -1, "output": "113fdb095d42d6e09327ab5b8df13fd8197a1eaf", - "actor_type": 3 + "actor_type": 4 }, { "address": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", @@ -76,7 +76,7 @@ "paused_height": -1, "unstaking_height": -1, "output": "3f52e08c4b3b65ab7cf098d77df5bf8cedcf5f99", - "actor_type": 3 + "actor_type": 4 }, { "address": "67eb3f0a50ae459fecf666be0e93176e92441317", @@ -87,7 +87,7 @@ "paused_height": -1, "unstaking_height": -1, "output": "67eb3f0a50ae459fecf666be0e93176e92441317", - "actor_type": 3 + "actor_type": 4 }, { "address": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", @@ -98,7 +98,7 @@ "paused_height": -1, "unstaking_height": -1, "output": "6f66574e1f50f0ef72dff748c3f11b9e0e89d32a", - "actor_type": 3 + "actor_type": 4 } ], "applications": [ From b1999e163fc12c582ef2e24b4a7e83ee2ec8a25d Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 9 Jan 2023 16:09:43 -0500 Subject: [PATCH 16/21] Replied to all comments --- consensus/e2e_tests/utils_test.go | 4 ++-- consensus/types/types.go | 2 +- docs/demos/iteration_3_end_to_end_tx.md | 17 +++++++++-------- shared/modules/doc/CHANGELOG.md | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/consensus/e2e_tests/utils_test.go b/consensus/e2e_tests/utils_test.go index 0467e1f5d..353da5372 100644 --- a/consensus/e2e_tests/utils_test.go +++ b/consensus/e2e_tests/utils_test.go @@ -82,7 +82,7 @@ func CreateTestConsensusPocketNodes( }) for i, runtimeMgr := range runtimeMgrs { pocketNode := CreateTestConsensusPocketNode(t, &runtimeMgr, eventsChannel) - // TODO(olshansky): Figure this part out. + // TODO(#434): Improve the use of NodeIDs pocketNodes[typesCons.NodeId(i+1)] = pocketNode } return @@ -105,7 +105,7 @@ func CreateTestConsensusPocketNodesNew( }) for i, runtimeMgr := range runtimeMgrs { pocketNode := CreateTestConsensusPocketNode(t, &runtimeMgr, eventsChannel) - // TODO(olshansky): Figure this part out. + // TODO(#434): Improve the use of NodeIDs pocketNodes[typesCons.NodeId(i+1)] = pocketNode } return diff --git a/consensus/types/types.go b/consensus/types/types.go index 263ea8862..3359a5eef 100644 --- a/consensus/types/types.go +++ b/consensus/types/types.go @@ -34,7 +34,7 @@ func GetValAddrToIdMap(validatorMap ValidatorMap) (ValAddrToIdMap, IdToValAddrMa valToIdMap := make(ValAddrToIdMap, len(valAddresses)) idToValMap := make(IdToValAddrMap, len(valAddresses)) for i, addr := range valAddresses { - nodeId := NodeId(i + 1) + nodeId := NodeId(i + 1) // TODO(#434): Improve the use of NodeIDs valToIdMap[addr] = nodeId idToValMap[nodeId] = addr } diff --git a/docs/demos/iteration_3_end_to_end_tx.md b/docs/demos/iteration_3_end_to_end_tx.md index 016c06c22..5727c2201 100644 --- a/docs/demos/iteration_3_end_to_end_tx.md +++ b/docs/demos/iteration_3_end_to_end_tx.md @@ -105,17 +105,18 @@ For the following steps, you'll need to use the accounts of the first two valida 1. You can just: - ```bash - echo '"6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4"' > /tmp/val1.json - echo '"5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb"' > /tmp/val2.json - ``` +```bash +echo '"6fd0bc54cc2dd205eaf226eebdb0451629b321f11d279013ce6fdd5a33059256b2eda2232ffb2750bf761141f70f75a03a025f65b2b2b417c7f8b3c9ca91e8e4"' > /tmp/val1.json + +echo '"5db3e9d97d04d6d70359de924bb02039c602080d6bf01a692bad31ad5ef93524c16043323c83ffd901a8bf7d73543814b8655aa4695f7bfb49d01926fc161cdb"' > /tmp/val2.json +``` 2. You can use `jq` and run these commands: - ```bash - cat ./build/config/config1.json | jq '.private_key' > /tmp/val1.json - cat ./build/config/config2.json | jq '.private_key' > /tmp/val2.json - ``` +```bash +cat ./build/config/config1.json | jq '.private_key' > /tmp/val1.json +cat ./build/config/config2.json | jq '.private_key' > /tmp/val2.json +``` 3. You can manually copy-paste the private keys from the config files into the `/tmp/val1.json` and `/tmp/val2.json` files. Remember to keep the double quotes around the private keys ("private_key" field in the JSON). diff --git a/shared/modules/doc/CHANGELOG.md b/shared/modules/doc/CHANGELOG.md index edf20d120..f7d629a94 100644 --- a/shared/modules/doc/CHANGELOG.md +++ b/shared/modules/doc/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.7] - 2023-01-03 +## [0.0.0.7] - 2023-01-09 - Added comments to the functions exposed by `P2PModule` From 0235e1e0050fda0f29e3ccc17ea8885bc1e68304 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Mon, 9 Jan 2023 16:29:45 -0500 Subject: [PATCH 17/21] Updated changelogs appropriately --- build/docs/CHANGELOG.md | 4 ++++ persistence/docs/CHANGELOG.md | 9 ++++----- runtime/docs/CHANGELOG.md | 4 ++++ utility/test/block_test.go | 4 ++-- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/build/docs/CHANGELOG.md b/build/docs/CHANGELOG.md index aa092940b..81b989529 100644 --- a/build/docs/CHANGELOG.md +++ b/build/docs/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.2] - 2023-01-09 + +- Reorder private keys so addresses (retrieved by transforming private keys) to reflect the numbering in LocalNet appropriately. The address for val1, based on config1, will have the lexicographically first address. This makes debugging easier. + ## [0.0.0.1] - 2023-01-03 - Removed `BaseConfig` from `configs` diff --git a/persistence/docs/CHANGELOG.md b/persistence/docs/CHANGELOG.md index fcc6d1888..d2540fda5 100644 --- a/persistence/docs/CHANGELOG.md +++ b/persistence/docs/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.19] - 2023-01-09 + +- Minor logging improvements + ## [0.0.0.18] - 2023-01-03 - Renamed `InitParams` to `InitGenesisParams` @@ -130,14 +134,12 @@ KVStore changes - Ported over storing blocks and block components to the Persistence module from Consensus and Utility modules - Encapsulated `TxIndexer` logic to the persistence context only - ## [0.0.0.9] - 2022-10-19 - Fixed `ToPersistenceActors()` by filling all structure fields - Deprecated `BaseActor` -> `Actor` - Changed default actor type to `ActorType_Undefined` - ## [0.0.0.8] - 2022-10-12 ### [#235](https://github.com/pokt-network/pocket/pull/235) Config and genesis handling @@ -155,8 +157,6 @@ KVStore changes - Don't ignore the exit code of `m.Run()` in the unit tests - Fixed several broken unit tests related to type casting - - ## [0.0.0.6] - 2022-09-30 - Removed no-op `DeleteActor` code @@ -166,7 +166,6 @@ KVStore changes - Added ticks to CHANGELOG.md - Removed reference to Utility Mod's `BigIntToString()` and used internal `BigIntToString()` - ## [0.0.0.5] - 2022-09-14 - Consolidated `PostgresContext` and `PostgresDb` into a single structure diff --git a/runtime/docs/CHANGELOG.md b/runtime/docs/CHANGELOG.md index 945232db7..7f274e6b9 100644 --- a/runtime/docs/CHANGELOG.md +++ b/runtime/docs/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.0.0.4] - 2023-01-09 + +- Updated tests to reflect the updated genesis file + ## [0.0.0.3] - 2023-01-03 - Split testing/development configs into separate files diff --git a/utility/test/block_test.go b/utility/test/block_test.go index 43e39dacc..043cc3d66 100644 --- a/utility/test/block_test.go +++ b/utility/test/block_test.go @@ -32,7 +32,7 @@ func TestUtilityContext_ApplyBlock(t *testing.T) { require.NoError(t, err) require.NotNil(t, appHash) - // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. + // // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. // beginBlock logic verify // missed, err := ctx.GetValidatorMissedBlocks(byzantine.Address) // require.NoError(t, err) @@ -82,7 +82,7 @@ func TestUtilityContext_BeginBlock(t *testing.T) { _, er = ctx.ApplyBlock() require.NoError(t, er) - // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. + // // TODO: Uncomment this once `GetValidatorMissedBlocks` is implemented. // beginBlock logic verify // missed, err := ctx.GetValidatorMissedBlocks(byzantine.Address) // require.NoError(t, err) From 096c3bed559c1204d23a7fefecec92bd4c8494bf Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Tue, 10 Jan 2023 18:45:16 -0500 Subject: [PATCH 18/21] Remove files added in prev commit --- consensus/helpers.go | 16 +-- p2p/addrbook_provider/persistence.go | 114 ------------------ shared/bus.go | 174 --------------------------- 3 files changed, 8 insertions(+), 296 deletions(-) delete mode 100644 p2p/addrbook_provider/persistence.go delete mode 100644 shared/bus.go diff --git a/consensus/helpers.go b/consensus/helpers.go index 02cc3327e..fd38af065 100644 --- a/consensus/helpers.go +++ b/consensus/helpers.go @@ -186,16 +186,16 @@ func (m *consensusModule) broadcastToValidators(msg *typesCons.HotstuffMessage) return } - if err := m.GetBus().GetP2PModule().Broadcast(anyConsensusMessage); err != nil { - m.nodeLogError(typesCons.ErrBroadcastMessage.Error(), err) - return + validators, err := m.getValidatorsAtHeight(m.CurrentHeight()) + if err != nil { + m.nodeLogError(typesCons.ErrPersistenceGetAllValidators.Error(), err) } - // for _, val := range m.validatorMap { - // if err := m.GetBus().GetP2PModule().Send(cryptoPocket.AddressFromString(val.GetAddress()), anyConsensusMessage); err != nil { - // m.nodeLogError(typesCons.ErrBroadcastMessage.Error(), err) - // } - // } + for _, val := range validators { + if err := m.GetBus().GetP2PModule().Send(cryptoPocket.AddressFromString(val.GetAddress()), anyConsensusMessage); err != nil { + m.nodeLogError(typesCons.ErrBroadcastMessage.Error(), err) + } + } } /*** Persistence Helpers ***/ diff --git a/p2p/addrbook_provider/persistence.go b/p2p/addrbook_provider/persistence.go deleted file mode 100644 index 32685776a..000000000 --- a/p2p/addrbook_provider/persistence.go +++ /dev/null @@ -1,114 +0,0 @@ -package addrbook_provider - -import ( - "fmt" - "log" - - "github.com/pokt-network/pocket/p2p/transport" - typesP2P "github.com/pokt-network/pocket/p2p/types" - "github.com/pokt-network/pocket/runtime/configs" - coreTypes "github.com/pokt-network/pocket/shared/core/types" - cryptoPocket "github.com/pokt-network/pocket/shared/crypto" - "github.com/pokt-network/pocket/shared/modules" -) - -var _ modules.IntegratableModule = &persistenceAddrBookProvider{} -var _ typesP2P.AddrBookProvider = &persistenceAddrBookProvider{} - -type persistenceAddrBookProvider struct { - bus modules.Bus - p2pCfg *configs.P2PConfig - connFactory typesP2P.ConnectionFactory -} - -func NewPersistenceAddrBookProvider(bus modules.Bus, p2pCfg *configs.P2PConfig, options ...func(*persistenceAddrBookProvider)) *persistenceAddrBookProvider { - pabp := &persistenceAddrBookProvider{ - bus: bus, - p2pCfg: p2pCfg, - connFactory: transport.CreateDialer, // default connection factory, overridable with WithConnectionFactory() - } - - for _, o := range options { - o(pabp) - } - - return pabp -} - -// WithConnectionFactory allows the user to specify a custom connection factory -func WithConnectionFactory(connFactory typesP2P.ConnectionFactory) func(*persistenceAddrBookProvider) { - return func(pabp *persistenceAddrBookProvider) { - pabp.connFactory = connFactory - } -} - -func (pabp *persistenceAddrBookProvider) GetBus() modules.Bus { - return pabp.bus -} - -func (pabp *persistenceAddrBookProvider) SetBus(bus modules.Bus) { - pabp.bus = bus -} - -func (pabp *persistenceAddrBookProvider) GetStakedAddrBookAtHeight(height uint64) (typesP2P.AddrBook, error) { - persistenceReadContext, err := pabp.GetBus().GetPersistenceModule().NewReadContext(int64(height)) - if err != nil { - return nil, err - } - stakedActors, err := persistenceReadContext.GetAllStakedActors(int64(height)) - if err != nil { - return nil, err - } - // TODO(#203): refactor `ValidatorMap` - validatorMap := make(modules.ValidatorMap, len(stakedActors)) - for _, v := range stakedActors { - validatorMap[v.GetAddress()] = *v - } - addrBook, err := pabp.ActorsToAddrBook(validatorMap) - if err != nil { - return nil, err - } - return addrBook, nil -} - -// TODO (#426): refactor so that it's possible to connect to peers without using the GenericParam and having to filter out non-validator actors. -// AddrBook and similar concepts shouldn't leak outside the P2P module. It should be possible to broadcast messages to all peers or only to a specific actor type -// without having to know the underlying implementation in the P2P module. -func (pabp *persistenceAddrBookProvider) ActorsToAddrBook(actors map[string]coreTypes.Actor) (typesP2P.AddrBook, error) { - book := make(typesP2P.AddrBook, 0) - for _, v := range actors { - // only add validator actors since they are the only ones having a service url in their generic param at the moment - if v.ActorType != coreTypes.ActorType_ACTOR_TYPE_VAL { - continue - } - networkPeer, err := pabp.ActorToNetworkPeer(v) - if err != nil { - log.Println("[WARN] Error connecting to validator:", err) - continue - } - book = append(book, networkPeer) - } - return book, nil -} - -// TODO (#426): refactor so that it doesn't use the GenericParam anymore to connect to the peer -func (pabp *persistenceAddrBookProvider) ActorToNetworkPeer(v coreTypes.Actor) (*typesP2P.NetworkPeer, error) { - conn, err := pabp.connFactory(pabp.p2pCfg, v.GetGenericParam()) // service url - if err != nil { - return nil, fmt.Errorf("error resolving addr: %v", err) - } - - pubKey, err := cryptoPocket.NewPublicKey(v.GetPublicKey()) - if err != nil { - return nil, err - } - - peer := &typesP2P.NetworkPeer{ - Dialer: conn, - PublicKey: pubKey, - Address: pubKey.Address(), - ServiceUrl: v.GetGenericParam(), // service url - } - - return peer, nil -} diff --git a/shared/bus.go b/shared/bus.go deleted file mode 100644 index ab9b54cfa..000000000 --- a/shared/bus.go +++ /dev/null @@ -1,174 +0,0 @@ -package shared - -import ( - "log" - - "github.com/pokt-network/pocket/shared/messaging" - "github.com/pokt-network/pocket/shared/modules" -) - -var _ modules.Bus = &bus{} - -type bus struct { - modules.Bus - - // Bus events - channel modules.EventsChannel - - // Modules - persistence modules.PersistenceModule - p2p modules.P2PModule - utility modules.UtilityModule - consensus modules.ConsensusModule - telemetry modules.TelemetryModule - logger modules.LoggerModule - rpc modules.RPCModule - - runtimeMgr modules.RuntimeMgr -} - -const ( - DefaultPocketBusBufferSize = 100 -) - -func CreateBus( - runtimeMgr modules.RuntimeMgr, - persistence modules.PersistenceModule, - p2p modules.P2PModule, - utility modules.UtilityModule, - consensus modules.ConsensusModule, - telemetry modules.TelemetryModule, - logger modules.LoggerModule, - rpc modules.RPCModule, -) (modules.Bus, error) { - bus := &bus{ - channel: make(modules.EventsChannel, DefaultPocketBusBufferSize), - - runtimeMgr: runtimeMgr, - - persistence: persistence, - p2p: p2p, - utility: utility, - consensus: consensus, - telemetry: telemetry, - logger: logger, - rpc: rpc, - } - - modules := map[string]modules.Module{ - "persistence": persistence, - "consensus": consensus, - "p2p": p2p, - "utility": utility, - "telemetry": telemetry, - "logger": logger, - "rpc": rpc, - } - - // checks if modules are not nil and sets their bus to this bus instance. - // will not carry forward if one of the modules is nil - for modName, mod := range modules { - if mod == nil { - log.Fatalf("Bus Error: the provided %s module is nil, Please use CreateBusWithOptionalModules if you intended it to be nil.", modName) - } - mod.SetBus(bus) - } - - return bus, nil -} - -// This is a version of CreateBus that accepts nil modules. -// This function allows you to use a specific module in isolation of other modules by providing a bus with nil modules. -// -// Example of usage: `app/client/main.go` -// -// We want to use the pre2p module in isolation to communicate with nodes in the network. -// The pre2p module expects to retrieve a telemetry module through the bus to perform instrumentation, thus we need to inject a bus that can retrieve a telemetry module. -// However, we don't need telemetry for the dev client. -// Using `CreateBusWithOptionalModules`, we can create a bus with only pre2p and a NOOP telemetry module -// so that we can the pre2p module without any issues. -func CreateBusWithOptionalModules( - runtimeMgr modules.RuntimeMgr, - persistence modules.PersistenceModule, - p2p modules.P2PModule, - utility modules.UtilityModule, - consensus modules.ConsensusModule, - telemetry modules.TelemetryModule, - logger modules.LoggerModule, - rpc modules.RPCModule, -) modules.Bus { - bus := &bus{ - channel: make(modules.EventsChannel, DefaultPocketBusBufferSize), - - runtimeMgr: runtimeMgr, - - persistence: persistence, - p2p: p2p, - utility: utility, - consensus: consensus, - telemetry: telemetry, - logger: logger, - rpc: rpc, - } - - maybeSetModuleBus := func(mod modules.Module) { - if mod != nil { - mod.SetBus(bus) - } - } - - maybeSetModuleBus(persistence) - maybeSetModuleBus(p2p) - maybeSetModuleBus(utility) - maybeSetModuleBus(consensus) - maybeSetModuleBus(telemetry) - maybeSetModuleBus(logger) - maybeSetModuleBus(rpc) - - return bus -} - -func (m *bus) PublishEventToBus(e *messaging.PocketEnvelope) { - m.channel <- e -} - -func (m *bus) GetBusEvent() *messaging.PocketEnvelope { - e := <-m.channel - return e -} - -func (m *bus) GetEventBus() modules.EventsChannel { - return m.channel -} - -func (m *bus) GetPersistenceModule() modules.PersistenceModule { - return m.persistence -} - -func (m *bus) GetP2PModule() modules.P2PModule { - return m.p2p -} - -func (m *bus) GetUtilityModule() modules.UtilityModule { - return m.utility -} - -func (m *bus) GetConsensusModule() modules.ConsensusModule { - return m.consensus -} - -func (m *bus) GetTelemetryModule() modules.TelemetryModule { - return m.telemetry -} - -func (m *bus) GetLoggerModule() modules.LoggerModule { - return m.logger -} - -func (m *bus) GetRPCModule() modules.RPCModule { - return m.rpc -} - -func (m *bus) GetRuntimeMgr() modules.RuntimeMgr { - return m.runtimeMgr -} From c8669f7335adeef55a2ce64bd3bb52e55d72eeb8 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Tue, 10 Jan 2023 19:10:08 -0500 Subject: [PATCH 19/21] Fix another test --- consensus/e2e_tests/pacemaker_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/consensus/e2e_tests/pacemaker_test.go b/consensus/e2e_tests/pacemaker_test.go index 87ad46f3e..ee9bd36e2 100644 --- a/consensus/e2e_tests/pacemaker_test.go +++ b/consensus/e2e_tests/pacemaker_test.go @@ -151,9 +151,6 @@ func TestPacemakerCatchupSameStepDifferentRounds(t *testing.T) { runtimeConfig.GetConfig().Consensus.PacemakerConfig.TimeoutMsec = paceMakerTimeoutMsec } - // Create & start test pocket nodes - StartAllTestPocketNodes(t, pocketNodes) - // Prepare leader info leaderId := typesCons.NodeId(3) require.Equal(t, uint64(leaderId), testHeight%numValidators) // Uses our deterministic round robin leader election From c47b125d8c3390de56b449930e05ec0225bf6704 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Tue, 10 Jan 2023 19:18:04 -0500 Subject: [PATCH 20/21] Fix config --- build/config/config2.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/config/config2.json b/build/config/config2.json index e4dbeaa88..1c5282277 100644 --- a/build/config/config2.json +++ b/build/config/config2.json @@ -25,7 +25,7 @@ "consensus_port": 8080, "use_rain_tree": true, "is_empty_connection_type": false, - "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2" + "private_key": "b37d3ba2f232060c41ba1177fea6008d885fcccad6826d64ee7d49f94d1dbc49a8b6be75d7551da093f788f7286c3a9cb885cfc8e52710eac5f1d5e5b4bf19b2", "max_mempool_count": 100000 }, "telemetry": { From 067336f421178a309ee1d0a9257f18fb9987f7a6 Mon Sep 17 00:00:00 2001 From: Daniel Olshansky Date: Tue, 10 Jan 2023 19:20:46 -0500 Subject: [PATCH 21/21] Updated changelogs appropriately --- build/docs/CHANGELOG.md | 2 +- consensus/doc/CHANGELOG.md | 2 +- p2p/CHANGELOG.md | 2 +- persistence/docs/CHANGELOG.md | 2 +- runtime/docs/CHANGELOG.md | 2 +- shared/CHANGELOG.md | 2 +- shared/modules/doc/CHANGELOG.md | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/build/docs/CHANGELOG.md b/build/docs/CHANGELOG.md index 2eb702380..93f725b94 100644 --- a/build/docs/CHANGELOG.md +++ b/build/docs/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.3] - 2023-01-10 +## [0.0.0.3] - 2023-01-11 - Reorder private keys so addresses (retrieved by transforming private keys) to reflect the numbering in LocalNet appropriately. The address for val1, based on config1, will have the lexicographically first address. This makes debugging easier. diff --git a/consensus/doc/CHANGELOG.md b/consensus/doc/CHANGELOG.md index b1ddc923f..8a9ddc5c2 100644 --- a/consensus/doc/CHANGELOG.md +++ b/consensus/doc/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.18] - 2023-01-10 +## [0.0.0.18] - 2023-01-11 ### Consensus - Core diff --git a/p2p/CHANGELOG.md b/p2p/CHANGELOG.md index 4f8f82e36..a2f1ef171 100644 --- a/p2p/CHANGELOG.md +++ b/p2p/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.18] - 2023-01-10 +## [0.0.0.18] - 2023-01-11 - Add a lock to the mempool to avoid parallel messages which has caused the node to crash in the past diff --git a/persistence/docs/CHANGELOG.md b/persistence/docs/CHANGELOG.md index fc1bd3207..46ef00a2c 100644 --- a/persistence/docs/CHANGELOG.md +++ b/persistence/docs/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.20] - 2023-01-10 +## [0.0.0.20] - 2023-01-11 - Minor logging improvements diff --git a/runtime/docs/CHANGELOG.md b/runtime/docs/CHANGELOG.md index e3f70266d..3049c0921 100644 --- a/runtime/docs/CHANGELOG.md +++ b/runtime/docs/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.6] - 2023-01-10 +## [0.0.0.6] - 2023-01-11 - Updated tests to reflect the updated genesis file diff --git a/shared/CHANGELOG.md b/shared/CHANGELOG.md index 8750487c8..2921cd08a 100644 --- a/shared/CHANGELOG.md +++ b/shared/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.11] - 2023-01-10 +## [0.0.0.11] - 2023-01-11 - Make the events channel hold pointers rather than copies of the message diff --git a/shared/modules/doc/CHANGELOG.md b/shared/modules/doc/CHANGELOG.md index f7d629a94..84f2420df 100644 --- a/shared/modules/doc/CHANGELOG.md +++ b/shared/modules/doc/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.0.7] - 2023-01-09 +## [0.0.0.7] - 2023-01-11 - Added comments to the functions exposed by `P2PModule`