-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Added Privacy Test Scenario #3 to privacy-simple-network.py #10253
Added Privacy Test Scenario #3 to privacy-simple-network.py #10253
Conversation
…d added getParticipantNum method.
…ck header state methods.
|
||
def getParticipantNum(self, nodeToIdentify): | ||
num = 0 | ||
for node in self.nodes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for num, node in zip(range(len(self.nodes)), self.nodes)
and dispense with manually initializing and incrementing num
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or alternatively we can do something like this:
range_nodes = range(len(self.nodes))
nodeDict = dict(zip(self.nodes, range_nodes)) | {self.biosNode : self.totalNodes}
return nodeDict[nodeToIndentify]
minBlocksForGuarantee = 2 | ||
assert producer not in self.getProducers() or blockNum - headBlockNum < minBlocksForGuarantee, \ | ||
"It is {} blocks past the block when we activated the features and block num: {} was produced by this \ | ||
node, so features should have been set." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing format call for block num substitutions.
# feature should be in block for this node's producers, if it is at least 2 blocks after we sent the activate | ||
minBlocksForGuarantee = 2 | ||
assert producer not in self.getProducers() or blockNum - headBlockNum < minBlocksForGuarantee, \ | ||
"It is {} blocks past the block when we activated the features and block num: {} was produced by this \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for colon after "block num" in message.
producer = block["producer"] | ||
producers[producer] += 1 | ||
assert lastProducer != producer or producers[producer] == 1, \ | ||
"We have already cycled through a complete cycle, so feature should have been set by now. \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "Producer schedule cycle completed and feature has not been set. Initial block num: {}, current block num: {}"
|
||
producer = block["producer"] | ||
producers[producer] += 1 | ||
assert lastProducer != producer or producers[producer] == 1, \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire assert doesn't seem very useful. It will trigger immediately in a single producer network, and never trigger in a multi-producer network because the second assert below will always trigger first.
# this is passed to limit the number of add/remove table entries are processed, but using it here to keep from getting duplicate transactions | ||
publishProcessNum = 20 | ||
def security_group(addNodeNums=[], removeNodeNums=[]): | ||
def createAction(nodeNums): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def createAction(nodeNums):
return None if len(nodeNums) == 0 else '[[{}]]'.format(','.join(['"{}"'.format(Node.participantName(nodeNum)) for nodeNum in nodeNums]))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those one liners are hard to read.
we can have at least something like that:
def createAction(nodeNums):
if len(nodeNums) == 0
return None
return '[[{}]]'.format(','.join(['"{}"'.format(Node.participantName(n)) for n in nodeNums]))
|
||
while not done and len(participants) > pnodes: | ||
publishTrans = remFromSg() | ||
Utils.Print("publishTrans: {}".format(json.dumps(publishTrans, indent=2))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason indent doesn't match the indent used elsewhere?
Missed Peer Review comment changes from #10253.
Change Description
Added more complex scenarios and covered the Test Scenario #3. Also made improvements to Node and Cluster and pulled in common code.
Change Type
Select ONE:
Testing Changes
Select ANY that apply:
Consensus Changes
API Changes
Documentation Additions