mobilecoinfoundation / mobilecoin

Private payments for mobile devices.
Other
1.16k stars 148 forks source link

mc-consensus-enclave crate lacks unit tests #2664

Closed cbeck88 closed 1 year ago

cbeck88 commented 1 year ago

Currently, we have a pretty good amount testing in mc-consensus-enclave-impl which is largely the business logic of the enclave, particularly, the parts around handling of blocks and transactions.

However, we do not have unit tests on the mc-consensus-enclave crate which is the API actually exposed to the untrusted side.

https://github.com/mobilecoinfoundation/mobilecoin/tree/1be70f94e7738f2c904ae18162dcfde9cf0acc22/consensus/enclave/src

Unit tests here could help ensure that AKE is working, that submitting AKE encrypted transactions is working as expected, that calls to form_block are working as expected.

It could make sense to take a bunch of the tests in the impl crate and try to make them generic over anything that implements trait ConsensusEnclave. Then in principle we could run them both on the impl business logic and on the fully-assembled enclave, and so gain greater confidence that the fully assembled enclave is working as expected. In this case if we see unit test failures on the assembled object but not the impl, then we would know there is probably something wrong in the plumbing between the impl and the untrusted interface.

Tests here could also help with developing fuzz tests for the enclave, since they could be used to provide an initial corpus of inputs.

cbeck88 commented 1 year ago

we now have some tests