Parameterize max hops and payload size - WIP #1
Parameterize max hops and payload size - WIP #1
Conversation
work in progress, needs fixing, tests failing
MaxHops int | ||
} | ||
|
||
func NewSphinxParams(maxHops, payloadSize int) *SphinxParams { |
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.
Needed? can't clients directly just &SphinxParams{payloadSize, maxHops}
?
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.
indeed. perhaps that's better.
copy(payload[:], ciphertextPayload) | ||
onionPacket := NewOnionReply(surb.Header, payload) | ||
return surb.FirstHop[:], onionPacket, nil | ||
sphinxPacket := NewOnionReply(surb.Header, ciphertextPayload) |
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.
Worth keeping an assertion that the payload is the expected PayloadSize
?
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.
hmm yeah... somewhere there should be such an assertion
@@ -139,6 +139,7 @@ func newTestRoute(numHops int) ([]*SphinxNode, *SphinxPacket, error) { | |||
route := make([][16]byte, len(nodes)) | |||
for i := 0; i < len(nodes); i++ { | |||
route[i] = nodes[i].id | |||
//fmt.Printf("node id %x\n", nodes[i].id) |
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.
Intentional?
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.
oops, yeah i should remove that.
nodeMap[nodes[i].id] = nodes[i] | ||
} | ||
pki := NewDummyPKI(nodeKeys) | ||
randReader, err := NewFixedNoiseReader("b5451d2eb2faf3f84bc4778ace6516e73e9da6c597e6f96f7e63c7ca6c9456018be9fd84883e4469a736c66fcaeceacf080fb06bc45859796707548c356c462594d1418b5349daf8fffe21a67affec10c0a2e3639c5bd9e8a9ddde5caf2e1db802995f54beae23305f2241c6517d301808c0946d5895bfd0d4b53d8ab2760e4ec8d4b2309eec239eedbab2c6ae532da37f3b633e256c6b551ed76321cc1f301d74a0a8a0673ea7e489e984543ca05fe0ff373a6f3ed4eeeaafd18292e3b182c25216aeb8") |
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.
comment for what this magic number is would be useful. How was it generated?
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.
ah ok, the mix client uses a entropy source for key generation and client id and message id generation. for tests i sometimes what to test those same code paths but i want it to be deterministic, so that's what the randReader thing is for.
ok i've tried to make all the corrections specified in your code review. clearly we should continue this line of code review and corrections on the main branch later once this feature branch is merged. |
Looks good. It's certainly easier to review on specific commits than full files, but will try to go through the current code as well. |
work in progress, needs fixing, tests failing