Interface for test infrastructure to inspect streams

[akoshelev]

This change implements #1195 by providing a means for unit tests to intercept send requests that send circuit-specific data between MPC helpers.

This change also updates the existing unit tests for malicious reshare and reveal protocols to showcase the API and test it in real scenarios. In both cases, using it, led to significant reduction in code redundancy. The key improvement is that now tests are not required to re-implement the basic protocol to inject additive attacks. It can now be done outside the circuit being tested.

The support for sharding exists at the API level, but no actual implementation allows intercepting traffic between shards. There is currently not a lot of sharded protocols, so this can be postponed until later.

[codecov]

Codecov Report

Attention: Patch coverage is 98.97260% with 3 lines in your changes missing coverage. Please review.

Project coverage is 92.15%. Comparing base (b930896) to head (6f74816).
Report is 1 commits behind head on main.

Files	Patch %	Lines
ipa-core/src/helpers/mod.rs	87.50%	1 Missing ⚠️
ipa-core/src/helpers/transport/mod.rs	80.00%	1 Missing ⚠️
ipa-core/src/test_fixture/world.rs	98.52%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1202      +/-   ##
==========================================
+ Coverage   92.13%   92.15%   +0.02%     
==========================================
  Files         196      197       +1     
  Lines       29553    29658     +105     
==========================================
+ Hits        27228    27331     +103     
- Misses       2325     2327       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andyleiserson]

This is really cool. Making it easier to write tests of malicious behavior means we'll write more of them.

One thing that we can't do with this API is simulate the case where a helper both sends a manipulated share value to another helper and changes its copy of the share to match. This doesn't seem to me like something we have to do, but it does change the properties of the testing in a way that might be worth thinking about. I think the previous reshare_with_additive_attack did simulate this, by corrupting part1, which is both sent to the peer and used in the calculation of the output share. Specifically for reshare, we could simulate this with the new infrastructure by corrupting the values sent in both directions between a pair of helpers the same way.

[andyleiserson]

Suggested change

println!("{shares:?}");

(If this is an unintentional leftover debug print.)

[akoshelev]

I left it intentionally to print the shares if test fails. Still can't decide whether this approach should be preferred or no - it does not pollute test output, but makes the code look messier than it should

[andyleiserson]

Is it important to have this associated type, vs. using InspectContext directly? I also wonder if there is some existing type like Addr in the transport layer that could be reused here.

It also seems like using Context in these names is potentially confusing. Maybe InspectState or StreamMetadata?

[akoshelev]

We could use Addr here, the tradeoff is people will be writing add.gate.unwrap() and figure out a way to get rid of generic I on Addr. I feel that we will eventually want to inspect cross-shard traffic in addition to cross-helper, so generics may not work well here.

I agree it is messier on the stream inspector implementation side, but makes the usage of it a bit simpler

[akoshelev]

One thing that we can't do with this API is simulate the case where a helper both sends a manipulated share value to another helper and changes its copy of the share to match.

yea that probably requires a different mechanism. I didn't think that checking validate on malicious helper is useful because they can be assumed to return whatever they want, but if we ever need to do that, we could either write a different protocol, corrupt the data on both ways as you suggested or get a handle into malicious validation, so we can change u and/or v before calling validate