Quorum strat update #92
Conversation
- prevent deadlocks in block maker/vote strategy - deprecate singleblockmaker flag - seed pseudoe random block maker/vote deadline strategy - prevent tx propagation to tx pool during synchronisation
| @@ -112,7 +112,7 @@ func NewProtocolManager(config *core.ChainConfig, singleMiner bool, networkId in | |||
| quitSync: make(chan struct{}), | |||
| raftMode: raftMode, | |||
| } | |||
| if singleMiner { | |||
| if assumeSyncedInitially { | |||
| manager.synced = uint32(1) | |||
There was a problem hiding this comment.
There are problems propagating txs from a non-blockmaker node, even if the flag --singleblockmaker is on. The temporal solution is implementing the patch indicated below:
There was a problem hiding this comment.
That should not be not be an issue for quorum. In quorum each blockmaker starts with the assumption that it is already synced with the chain. If a node finds a node with a better chain it temporary stops accepting transactions until it has caught up.
This isn't an optimal solution and has corner cases that might explain why your transactions aren't propagated. Especially in small networks or networks with bad connectivity. It would help to determine what the problem is if you describe what your network setup looks like and what you are seeing.
There was a problem hiding this comment.
@hermanjunge have you experienced the reported issue with this PR? I have multiple tests sending transactions across multiple nodes with different roles and I didn't see a TX propagation issue.
|
Any outstanding concerns? @hermanjunge? |
| @@ -223,42 +237,67 @@ func (bv *BlockVoting) run(strat BlockMakerStrategy) { | |||
|
|
|||
| switch e := event.Data.(type) { | |||
| case downloader.StartEvent: // begin synchronising, stop block creation and/or voting | |||
| bv.synced = false | |||
| strat.Pause() | |||
| strat.PauseBlockMaking() | |||
There was a problem hiding this comment.
Please pause voting here too since it has been separated from block voting.
| @@ -13,27 +13,27 @@ contract BlockVoting { | |||
| // Raised when a new address is allowed to vote. | |||
There was a problem hiding this comment.
In the vote event, please change the name of the param from "blockNumber" to "height" to match the name of the expected value.
core/quorum/vote_strategy.go
Outdated
| maxBlockTime: int(maxBlockTime), | ||
| minVoteTime: int(minVoteTime), | ||
| maxVoteTime: int(maxVoteTime), | ||
| blockCreateActive: true, |
There was a problem hiding this comment.
The blockCreateActive and voteActive flags are always set to true, can the default values be based on the role of the node? e.g: if the node is configured for voting, then voteActive will be true, otherwise false. I think the voting and block creation timers should only be activated based on the role of the node.
- stop voting when chain synchronizes - rename parameter in Vote vent - immediatly vote when new block is imported - activate vote/block creation when node is configured for a particular role
| } | ||
| s.deadlineTimer = time.NewTimer(time.Duration(s.min+rand.Intn(s.max-s.min)) * time.Second) | ||
|
|
||
| s.voteTimer = time.NewTimer(time.Duration(s.minBlockTime+rand.Intn(s.maxVoteTime-s.minVoteTime)) * time.Second) |
There was a problem hiding this comment.
@bas-vk based on my earlier suggestion, can you please create the timer for voting and block making only if the node is configured with votingActive or blockCreateActive? I don't see a reason why the timer should be active if the node can't vote or create blocks.
|
@bas-vk , closing this for now. Please submit a new one once we are done testing the latest updates. |
This PR includes:
getCanonHash, fixes BlockVoting contract getCanonHash function exception #44