Uploaded image for project: 'Corda'
  1. CORDA-2817

TransactionBuilder can build unverifiable transactions in V4 if more than one cordapp loaded

    Details

    • Type: Bug
    • Status: Done (View workflow)
    • Priority: High
    • Resolution: Done
    • Affects versions: Corda 4, Corda Enterprise 4 RC05
    • Fix versions: Corda 4.1
    • Components: None
    • Labels:
      None

      Description

      This is due to a change to "flatten" commands in V4. i.e. combine commands added to transaction builder that only differ by the signers list into a single command with combined signers. I'm not sure I follow the logic for having done this fully. I would have thought contracts could have been modified to cope with this scenario, rather than changing TransactionBuilder, but perhaps not? If we can conclude we should revert, that would be a superior solution.

      Currently, TransactionBuilder.kt:677 is not the correct expression:

      CordappResolver.currentTargetVersion >= CORDA_VERSION_THAT_INTRODUCED_FLATTENED_COMMANDS

      This is flawed because it looks at the flow, but also doesn't work if there are any conflicting cordapps. It turns out this is the case if you run:

      ./gradlew clean :finance:contracts:test

      This results in two test failures because transactions do not verify due to behaviour change in TransactionBuilder due to the cordapps installed when running on V4.

      This can happen (i.e. we unpredictably stop flattening) in the wild on V4 if apps install overlapping classes. e.g. tokens SDK.

      In a discussion with Tudor Malene we discussed how the correct thing to do here is to group a particular command class dependent on the target version of the contract attached to the transaction, as it is the contract that demands grouping (or not) of the commands based on the target version of the contract. That all needs to be done during `toWireTransaction()` as we resolve the attachments. We'll need an internal `commands()` replacement, and the plumbing is significant.

      Unfortunately there seems no way to correctly implement the `commands()` method on the public API now that V4 has shipped, without it doing some heavyweight introspection of the attachments, which it is unable to do. Given the API describes what is supposed to happen, I think we should be able to improve the "what is the target version of the current cordapp" a little better, as really this method will only make sense in tests outside of a flow (in which case target version is current platform version, unless specified in a new parameter to MockServices or something), or somehow get there from the current flow (sub flow).

        Attachments

          Issue links

            Activity

              People

              • Assignee:
                Rick.Parker Rick Parker
                Reporter:
                Rick.Parker Rick Parker
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: