search

R3-Archive / obligation-cordapp

This repository is deprecated. The latest version of all the official CorDapp samples can be found in the official samples repository: https://github.com/corda/samples.
Other
5 stars 14 forks source link

Bug related to confidential identities #5

Open mikehearn opened 6 years ago

mikehearn commented 6 years ago

Reported by Alex Koller:

Playing with the Corda obligation example from here: https://github.com/corda/obligation-cordapp I think there's a bug. Provided I'm using anonymisation: if Party A issues obligation to Party B and then Party B transfers the obligation to Party C then Party A can no longer settle the obligation because it can't resolve the anonymised holder of the obligation (Party C). My question is, is it a bug in the example as it looks to be? And more importantly, in order to fix it, at which point should Party C introduce itself to the issuer (Party A)? When Party C signs the transfer from Party B through IdentitySyncFlow? But Party C won't be able to deanonymise Party A! So, how are Party A and C meant to be introduced to each other so that the obligation can settle eventually?

cxyzhang0 commented 6 years ago

I would think the introduction between A and C should happen during the transfer, similarly to how B introduces itself to A and C via IdentitySyncFlow. This is how it may work -

  1. In the TransferObligation$Initiator flow, just before (could be after too) B does IdentitySynchFlow.Send, sendAndReceive a payload of Triple(borrower, newlender, ptx.tx)
  2. In the TransferObligation$Responder flow, sendAndReceive the above payload. Depending on whether the responder is the borrower or the newlender, do a version of IdentitySyncFlow to the other.

I did the above experiment and made it work as desired, but not without making some changes to the IdentitySyncFlow. First, I needed to wrap it up into a stand-alone flow with its own flow session instead of using the one from TransferObligation$Initiator. That makes it is easy to reason about the messages back and forth.

Second, the original IdentitySyncFlow will throw a TransactionResolution exception because C does not know about the input state yet. I tried to use ResolveTransactionFlow on C, but that flow in Corda internal and for some reason I got net.corda.core.flows.UnexpectedFlowEndException: Counterparty sent session rejection message at unexpected time with message class net.corda.examples.obligation.flows.TransferObligation$Responder is not registered. Without relying on transaction resolution, I ended up by modifying the original IdentitySyncFlow to use outputs only, which is fine because both A and C are in the output.

So, the first change to IdentitySyncFlow is within each reach, which can be done in obligation-cordapp proper. BTW, that may be a desired variation any way in corda core. The second change requires PR on the corda core - basically change the primary constructor to take input stateRefs and output contract states instead of a WireTransaction. Let a secondary constructor take a WireTransction.

cxyzhang0 commented 6 years ago

I just realized that a less abrasive/disruptive change to IdentitySyncFlow is to only load an input state if it is available in local storage. I think that is a reasonable change and it makes IdentitySynchFlow more usable, e.g., party C in our case would be able to use it to introduce itself to A. I will see if I can raise a PR for review.

cxyzhang0 commented 6 years ago

I have raised the two PRs for the Corda codebase to help resolve the current issue.