fabric-private-chaincode icon indicating copy to clipboard operation
fabric-private-chaincode copied to clipboard

Missing status checks for transaction commit in tests

Open bvavala opened this issue 6 years ago • 2 comments

In many tests, transaction invocations are triggered with the waitForEvent flag, to block and wait for a notification of the commit event. When the event is delivered, so is the response. However, though the tests check the response, they do not check the commit status. E.g.:

2019-10-17 07:46:40.240 PDT [chaincodeCmd] ClientWait -> INFO 001 txid [f3df20192dac0a37d29b5761010a0c109a023b16862ad9c28d4e292e6f7a62e5] committed with status (VALID) at
2019-10-17 07:46:40.240 PDT [chaincodeCmd] ClientWait -> INFO 001 txid [6a6b733c997dce33862bbf4a880f5998df921b917f4339f5a7bc06a216d2e124] committed with status (MVCC_READ_CONFLICT) at

Considerations:

  • after a bad commit, many subsequent test iterations would fail, because expected results rely on data which is expected to be committed. The failure would however be only detected later, on the 'next' iteration, if any.
  • currently, tests are serialized, so it is unlikely that a commit status will be different than VALID
  • additional tests with concurrent transactions on shared keys would highlight the problem
  • the response "might" contain sensitive data despite a possible bad commit status. In this aspect, the issue is related to #182 , according to which the actual response should not be revealed before a successful commit in some situations.
  • this raises the question of what to do after the bad commit. In particular, whether the submitter should return unsuccessful or retry a few times.
  • this is issue is related though orthogonal to #104

bvavala avatar Oct 17 '19 22:10 bvavala

Interesting observation. Indeed the design of the auction implementation addresses those read/write conflicts by write each new bid to it's own key and thereby don't create conflicts.

The normal Fabric-way to resolve such a conflict conflict is to let the client resubmit (i.e. invoke the chaincode again and let the endorers do the work again) the transaction again. Depending on the application design this approach may take some time to get the transaction successfully committed.

FPC should not solve this problem, but indeed make sure that we don't leak any sensitive information.

mbrandenburger avatar Oct 22 '19 13:10 mbrandenburger

FPC should not solve this problem, but indeed make sure that we don't leak any sensitive information.

I think the issue here is that our return value check functions as used, e.g., in the test programs, don't check for such conflicts. So we might ignore errors ....

g2flyer avatar Oct 22 '19 15:10 g2flyer