Closed ggutoski closed 3 years ago
The nontrivial reviewer comments are:
src/gg20/sign/type5_common.rs
with the refactored code.tests/integration/single_thread/malicious/sign_delta_inv.rs
. It's a very quick-and-dirty solution---in a better world we'd clean up our test code. The old unit test is now redundant. I left it in the code base for now but we can delete it if desired.Recall that we've settled on a strategy for malicious tests whereby we embed malicious code directly into the protocol. Unfortunately we cannot follow this strategy for the delta-inverse tests because those tests require a rushing adversary and it is difficult to implement such an adversary within that strategy. Thus, for the delta-inverse test I resorted to the method of tweaking messages post hoc (after the round is complete). That method is acceptable for this particular task because the delta-inverse tests are amenable to it. But we might need to revisit our test architecture in the future if we ever encounter a situation with a rushing adversary that is not so amenable to the post hoc method.
Forgot to mention: I made the gamma_i
and Gamma_i
checks mandatory in type5_checks()
, even when type-5 sad path is entered via delta-inverse path:
https://github.com/axelarnetwork/tofn/blob/7be0e76c081b8ca263ae51d23a29dbcb2a774a42/src/gg20/sign/type5_common.rs#L106-L108
I did this because delta
depends on gamma_i
and so we always need to check gamma_i
, too. If type-5 is entered via delta-inverse then we don't have gamma_i
yet (nor Gamma_i
). Thus, I made it so that the bcasts_in
arg to type5_checks
is now a pair (r4::BcastHappy, BcastSadType5)
. If this code path is entered from round 4 then round 4 needs to compute both happy and sad path bcasts for the bcasts_in
arg.
fix #110
Notes
malicious_delta_inverse
for this sad path insrc/gg20/sign/tests.rs
. This test breaks with the pattern of all other tests for malicious behaviour, which are integration tests (not unit tests) located attests/integration/single_thread/malicious
. I did this because this delta-inverse sad path can only be triggered by a rushing adversary who tweaks her round-3 bcasts in response to messages from other parties. This requires visibility of the round-3 bcast struct. This struct is not part of the crate API and so is not visible from integration tests. I do not want to expose this struct in the crate API merely to facilitate a single integration test, so I made it a unit test instead.Gamma_i
because it's not yet available at this point in the protocol). It should be possible (though perhaps tedious) to refactor some common code here but I did not take the time to do so. I'm happy to discuss.