test(gossipsub): Signature flags tests #1496
Conversation
| type NodeConfig = tuple[sign: bool, verify: bool, anonymize: bool] | ||
| let scenarios = | ||
| @[ | ||
| # (sender_config, receiver_config, should_work) | ||
|
|
||
| # valid combos | ||
| # S default, R default | ||
| ((true, true, false), (true, true, false), true), | ||
| # S default, R anonymous | ||
| ((true, true, false), (false, false, true), true), | ||
| # S anonymous, R anonymous | ||
| ((false, false, true), (false, false, true), true), | ||
| # S only sign, R only verify | ||
| ((true, false, false), (false, true, false), true), | ||
| # S only verify, R only sign | ||
| ((true, true, true), (false, false, false), true), | ||
| # S anonymous (not signed despite the flag), R minimal | ||
| ((false, true, true), (true, false, false), true), | ||
| # S unsigned, R unsigned | ||
| ((false, false, false), (false, false, false), true), | ||
|
|
||
| # invalid combos | ||
| # S anonymous, R default | ||
| ((false, false, true), (true, true, false), false), | ||
| # S unsigned, R anonymous but verify | ||
| ((false, false, false), (true, true, true), false), | ||
| # S unsigned, R default | ||
| ((false, false, false), (true, true, false), false), | ||
| ] | ||
|
|
||
| for scenario in scenarios: | ||
| let title = "Compatibility matrix: " & $scenario | ||
| asyncTest title: | ||
| let | ||
| (senderConfig, receiverConfig, shouldWork) = scenario |
There was a problem hiding this comment.
would be better if scenarios is seq[Scenario], then type Scenario has senderConfig receiverConfig and shouldWork properties. senderConfig and receiverConfig are i guess NodeConfig ??
There was a problem hiding this comment.
Changed in 0c2b351, but I don't love how verbose it is now.
| gossip = true, | ||
| sign = senderConfig[0], | ||
| verifySignature = senderConfig[1], | ||
| anonymize = senderConfig[2], | ||
| )[0] | ||
| receiver = generateNodes( | ||
| 1, | ||
| gossip = true, | ||
| sign = receiverConfig[0], | ||
| verifySignature = receiverConfig[1], | ||
| anonymize = receiverConfig[2], |
There was a problem hiding this comment.
sign = senderConfig.sign,
verifySignature = senderConfig.verifySignature,
anonymize = senderConfig.anonymize
| var messageReceived = false | ||
| proc handler(topic: string, data: seq[byte]) {.async.} = | ||
| messageReceived = true |
There was a problem hiding this comment.
i guess we had some utility for this?
| check: | ||
| receivedMessages[0].data == testData | ||
| receivedMessages[0].fromPeer.data.len == 0 | ||
| receivedMessages[0].seqno.len == 0 | ||
| receivedMessages[0].signature.len == 0 | ||
| receivedMessages[0].key.len == 0 |
There was a problem hiding this comment.
check isAnonymusMessage(receivedMessages[0])
proc isAnonymusMessage(msg): bool =
return msg.fromPeer.data.len == 0 and msg.seqno.len == 0 ......
There was a problem hiding this comment.
code like this is much more self explanatory
There was a problem hiding this comment.
I don't agree here, adding proc isAnonymusMessage would be redundant if I were to use it in only one place. The test case is clear, it shows sending an anonymous message and asserts it's properties.
| const | ||
| libp2p_pubsub_sign {.booldefine.} = true | ||
| libp2p_pubsub_verify {.booldefine.} = true | ||
| libp2p_pubsub_anonymize {.booldefine.} = false |
There was a problem hiding this comment.
not sure if we had opened issue to remove these consts or we just discussed for removing them? anyway if there is issue let's close it
There was a problem hiding this comment.
We talked about it and I created this issue: #1434, which is linked.
Changes:
sign,verifySignatureandanonymizetestgossipsubskipmcachetests from PR were not working and were not included to run in CI