Address PR 465 comments

core/src/main/kotlin/net/corda/protocols/BroadcastTransactionProtocol.kt

@@ -15,10 +15,6 @@ import net.corda.core.transactions.SignedTransaction
  * @param participants a list of participants involved in the transaction.
  * @return a list of participants who were successfully notified of the transaction.
  */
-// TODO: Event needs to be replaced with something that's meaningful, but won't ever contain sensitive
-//       information (such as internal details of an account to take payment from). Suggest
-//       splitting ClientToServiceCommand into public and private parts, with only the public parts
-//       relayed here.
 class BroadcastTransactionProtocol(val notarisedTransaction: SignedTransaction,
                                    val participants: Set<Party>) : ProtocolLogic<Unit>() {
 

core/src/main/kotlin/net/corda/protocols/FinalityProtocol.kt

@@ -11,14 +11,9 @@ import net.corda.core.utilities.ProgressTracker
  * Finalise a transaction by notarising it, then recording it locally, and then sending it to all involved parties.
  *
  * @param transaction to commit.
- * @param events information on the event(s) which triggered the transaction.
  * @param participants a list of participants involved in the transaction.
  * @return a list of participants who were successfully notified of the transaction.
  */
-// TODO: Event needs to be replaced with something that's meaningful, but won't ever contain sensitive
-//       information (such as internal details of an account to take payment from). Suggest
-//       splitting ClientToServiceCommand into public and private parts, with only the public parts
-//       relayed here.
 class FinalityProtocol(val transaction: SignedTransaction,
                        val participants: Set<Party>,
                        override val progressTracker: ProgressTracker = tracker()): ProtocolLogic<Unit>() {

finance/src/main/kotlin/net/corda/protocols/CashProtocol.kt

@@ -11,15 +11,18 @@ import net.corda.core.protocols.StateMachineRunId
 import net.corda.core.serialization.OpaqueBytes
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.transactions.TransactionBuilder
-import org.slf4j.LoggerFactory
 import java.security.KeyPair
 import java.util.*
 
-class CashProtocol(val command: CashCommand): ProtocolLogic<TransactionBuildResult>() {
+/**
+ * Initiates a protocol that produces an Issue/Move or Exit Cash transaction.
+ *
+ * @param command Indicates what Cash transaction to create with what parameters.
+ */
+class CashProtocol(val command: CashCommand): ProtocolLogic<CashProtocolResult>() {
 
     @Suspendable
-    override fun call(): TransactionBuildResult {
+    override fun call(): CashProtocolResult {
-        LoggerFactory.getLogger("DEBUG").warn("CashProtocol call()ed with $command")
         return when (command) {
             is CashCommand.IssueCash -> issueCash(command)
             is CashCommand.PayCash -> initiatePayment(command)
@@ -27,8 +30,9 @@ class CashProtocol(val command: CashCommand): ProtocolLogic<TransactionBuildResu
         }
     }
 
+    // TODO check with the recipient if they want to accept the cash.
     @Suspendable
-    private fun initiatePayment(req: CashCommand.PayCash): TransactionBuildResult {
+    private fun initiatePayment(req: CashCommand.PayCash): CashProtocolResult {
         val builder: TransactionBuilder = TransactionType.General.Builder(null)
         // TODO: Have some way of restricting this to states the caller controls
         try {
@@ -43,18 +47,18 @@ class CashProtocol(val command: CashCommand): ProtocolLogic<TransactionBuildResu
             val tx = spendTX.toSignedTransaction(checkSufficientSignatures = false)
             val protocol = FinalityProtocol(tx, setOf(req.recipient))
             subProtocol(protocol)
-            return TransactionBuildResult.ProtocolStarted(
+            return CashProtocolResult.Success(
                     psm.id,
                     tx,
                     "Cash payment transaction generated"
             )
         } catch(ex: InsufficientBalanceException) {
-            return TransactionBuildResult.Failed(ex.message ?: "Insufficient balance")
+            return CashProtocolResult.Failed(ex.message ?: "Insufficient balance")
         }
     }
 
     @Suspendable
-    private fun exitCash(req: CashCommand.ExitCash): TransactionBuildResult {
+    private fun exitCash(req: CashCommand.ExitCash): CashProtocolResult {
         val builder: TransactionBuilder = TransactionType.General.Builder(null)
         try {
             val issuer = PartyAndReference(serviceHub.myInfo.legalIdentity, req.issueRef)
@@ -78,18 +82,18 @@ class CashProtocol(val command: CashCommand): ProtocolLogic<TransactionBuildResu
             // Commit the transaction
             val tx = builder.toSignedTransaction(checkSufficientSignatures = false)
             subProtocol(FinalityProtocol(tx, participants))
-            return TransactionBuildResult.ProtocolStarted(
+            return CashProtocolResult.Success(
                     psm.id,
                     tx,
                     "Cash destruction transaction generated"
             )
         } catch (ex: InsufficientBalanceException) {
-            return TransactionBuildResult.Failed(ex.message ?: "Insufficient balance")
+            return CashProtocolResult.Failed(ex.message ?: "Insufficient balance")
         }
     }
 
     @Suspendable
-    private fun issueCash(req: CashCommand.IssueCash): TransactionBuildResult {
+    private fun issueCash(req: CashCommand.IssueCash): CashProtocolResult {
         val builder: TransactionBuilder = TransactionType.General.Builder(notary = null)
         val issuer = PartyAndReference(serviceHub.myInfo.legalIdentity, req.issueRef)
         Cash().generateIssue(builder, req.amount.issuedBy(issuer), req.recipient.owningKey, req.notary)
@@ -98,7 +102,7 @@ class CashProtocol(val command: CashCommand): ProtocolLogic<TransactionBuildResu
         val tx = builder.toSignedTransaction(checkSufficientSignatures = true)
         // Issuance transactions do not need to be notarised, so we can skip directly to broadcasting it
         subProtocol(BroadcastTransactionProtocol(tx, setOf(req.recipient)))
-        return TransactionBuildResult.ProtocolStarted(
+        return CashProtocolResult.Success(
                 psm.id,
                 tx,
                 "Cash issuance completed"
@@ -120,7 +124,6 @@ sealed class CashCommand {
      * to use the single byte "0x01" as a default.
      * @param recipient the party to issue the cash to.
      * @param notary the notary to use for this transaction.
-     * @param id the ID to be provided in events resulting from this request.
      */
     class IssueCash(val amount: Amount<Currency>,
                     val issueRef: OpaqueBytes,
@@ -132,7 +135,6 @@ sealed class CashCommand {
      *
      * @param amount the amount of currency to issue on to the ledger.
      * @param recipient the party to issue the cash to.
-     * @param id the ID to be provided in events resulting from this request.
      */
     class PayCash(val amount: Amount<Issued<Currency>>, val recipient: Party) : CashCommand()
 
@@ -141,29 +143,23 @@ sealed class CashCommand {
      *
      * @param amount the amount of currency to exit from the ledger.
      * @param issueRef the reference previously specified on the issuance.
-     * @param id the ID to be provided in events resulting from this request.
      */
     class ExitCash(val amount: Amount<Currency>, val issueRef: OpaqueBytes) : CashCommand()
 }
 
-sealed class TransactionBuildResult {
+sealed class CashProtocolResult {
     /**
-     * State indicating that a protocol is managing this request, and that the client should track protocol state machine
+     * @param transaction the transaction created as a result, in the case where the protocol completed successfully.
-     * updates for further information. The monitor will separately receive notification of the state machine having been
-     * added, as it would any other state machine. This response is used solely to enable the monitor to identify
-     * the state machine (and its progress) as associated with the request.
-     *
-     * @param transaction the transaction created as a result, in the case where the protocol has completed.
      */
-    class ProtocolStarted(val id: StateMachineRunId, val transaction: SignedTransaction?, val message: String?) : TransactionBuildResult() {
+    class Success(val id: StateMachineRunId, val transaction: SignedTransaction?, val message: String?) : CashProtocolResult() {
-        override fun toString() = "Started($message)"
+        override fun toString() = "Success($message)"
     }
 
     /**
      * State indicating the action undertaken failed, either directly (it is not something which requires a
      * state machine), or before a state machine was started.
      */
-    class Failed(val message: String?) : TransactionBuildResult() {
+    class Failed(val message: String?) : CashProtocolResult() {
         override fun toString() = "Failed($message)"
     }
 }

node/src/main/kotlin/net/corda/node/internal/CordaRPCOpsImpl.kt

@@ -15,6 +15,7 @@ import net.corda.core.toObservable
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.utilities.ProgressTracker
 import net.corda.node.services.messaging.*
+import net.corda.node.services.startProtocolPermission
 import net.corda.node.services.statemachine.ProtocolStateMachineImpl
 import net.corda.node.services.statemachine.StateMachineManager
 import net.corda.node.utilities.databaseTransaction
@@ -78,8 +79,9 @@ class CordaRPCOpsImpl(
         }
     }
 
-    override fun <T: Any> startProtocolGeneric(logicType: Class<out ProtocolLogic<T>>, vararg args: Any?): ProtocolHandle<T> {
+    // TODO: Check that this protocol is annotated as being intended for RPC invocation
-        requirePermission(logicType.name)
+    override fun <T: Any> startProtocolDynamic(logicType: Class<out ProtocolLogic<T>>, vararg args: Any?): ProtocolHandle<T> {
+        requirePermission(startProtocolPermission(logicType))
         val stateMachine = services.invokeProtocolAsync(logicType, *args) as ProtocolStateMachineImpl<T>
         return ProtocolHandle(
                 id = stateMachine.id,

node/src/main/kotlin/net/corda/node/services/RPCUserService.kt

@@ -41,4 +41,5 @@ data class User(val username: String, val password: String, val permissions: Set
     override fun toString(): String = "${javaClass.simpleName}($username, permissions=$permissions)"
 }
 
-inline fun <reified P : ProtocolLogic<*>> startProtocolPermission(): String = P::class.java.name
+fun <P : ProtocolLogic<*>> startProtocolPermission(clazz: Class<P>) = "StartProtocol.${clazz.name}"
+inline fun <reified P : ProtocolLogic<*>> startProtocolPermission(): String = startProtocolPermission(P::class.java)

node/src/main/kotlin/net/corda/node/services/messaging/CordaRPCOps.kt

@@ -96,7 +96,7 @@ interface CordaRPCOps : RPCOps {
      * result of running the protocol.
      */
     @RPCReturnsObservables
-    fun <T: Any> startProtocolGeneric(logicType: Class<out ProtocolLogic<T>>, vararg args: Any?): ProtocolHandle<T>
+    fun <T: Any> startProtocolDynamic(logicType: Class<out ProtocolLogic<T>>, vararg args: Any?): ProtocolHandle<T>
 
     /**
      * Returns Node's identity, assuming this will not change while the node is running.
@@ -115,7 +115,7 @@ interface CordaRPCOps : RPCOps {
 }
 
 /**
- * These allow type safe invocations of protocols, e.g.:
+ * These allow type safe invocations of protocols from Kotlin, e.g.:
  *
  * val rpc: CordaRPCOps = (..)
  * rpc.startProtocol(::ResolveTransactionsProtocol, setOf<SecureHash>(), aliceIdentity)
@@ -126,25 +126,25 @@ interface CordaRPCOps : RPCOps {
 inline fun <T : Any, reified R : ProtocolLogic<T>> CordaRPCOps.startProtocol(
         @Suppress("UNUSED_PARAMETER")
         protocolConstructor: () -> R
-) = startProtocolGeneric(R::class.java)
+) = startProtocolDynamic(R::class.java)
 inline fun <T : Any, A, reified R : ProtocolLogic<T>> CordaRPCOps.startProtocol(
         @Suppress("UNUSED_PARAMETER")
         protocolConstructor: (A) -> R,
         arg0: A
-) = startProtocolGeneric(R::class.java, arg0)
+) = startProtocolDynamic(R::class.java, arg0)
 inline fun <T : Any, A, B, reified R : ProtocolLogic<T>> CordaRPCOps.startProtocol(
         @Suppress("UNUSED_PARAMETER")
         protocolConstructor: (A, B) -> R,
         arg0: A,
         arg1: B
-) = startProtocolGeneric(R::class.java, arg0, arg1)
+) = startProtocolDynamic(R::class.java, arg0, arg1)
 inline fun <T : Any, A, B, C, reified R: ProtocolLogic<T>> CordaRPCOps.startProtocol(
         @Suppress("UNUSED_PARAMETER")
         protocolConstructor: (A, B, C) -> R,
         arg0: A,
         arg1: B,
         arg2: C
-) = startProtocolGeneric(R::class.java, arg0, arg1, arg2)
+) = startProtocolDynamic(R::class.java, arg0, arg1, arg2)
 inline fun <T : Any, A, B, C, D, reified R : ProtocolLogic<T>> CordaRPCOps.startProtocol(
         @Suppress("UNUSED_PARAMETER")
         protocolConstructor: (A, B, C, D) -> R,
@@ -152,7 +152,7 @@ inline fun <T : Any, A, B, C, D, reified R : ProtocolLogic<T>> CordaRPCOps.start
         arg1: B,
         arg2: C,
         arg3: D
-) = startProtocolGeneric(R::class.java, arg0, arg1, arg2, arg3)
+) = startProtocolDynamic(R::class.java, arg0, arg1, arg2, arg3)
 
 data class ProtocolHandle<A>(
         val id: StateMachineRunId,

node/src/main/kotlin/net/corda/node/services/messaging/RPCStructures.kt

@@ -28,7 +28,7 @@ import net.corda.core.serialization.*
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.transactions.WireTransaction
 import net.corda.node.services.User
-import net.corda.protocols.TransactionBuildResult
+import net.corda.protocols.CashProtocolResult
 import net.i2p.crypto.eddsa.EdDSAPrivateKey
 import net.i2p.crypto.eddsa.EdDSAPublicKey
 import org.objenesis.strategy.StdInstantiatorStrategy
@@ -174,8 +174,8 @@ private class RPCKryo(observableSerializer: Serializer<Observable<Any>>? = null)
         register(Cash.Clauses.ConserveAmount::class.java)
         register(listOf(Unit).javaClass) // SingletonList
         register(setOf(Unit).javaClass) // SingletonSet
-        register(TransactionBuildResult.ProtocolStarted::class.java)
+        register(CashProtocolResult.Success::class.java)
-        register(TransactionBuildResult.Failed::class.java)
+        register(CashProtocolResult.Failed::class.java)
         register(ServiceEntry::class.java)
         register(NodeInfo::class.java)
         register(PhysicalLocation::class.java)

node/src/test/kotlin/com/r3corda/node/services/RPCUserServiceImplTest.kt

@@ -34,7 +34,7 @@ class RPCUserServiceImplTest {
     @Test
     fun `single permission, which is in lower case`() {
         val service = loadWithContents("rpcUsers : [{ user=user1, password=letmein, permissions=[cash] }]")
-        assertThat(service.getUser("user1")?.permissions).containsOnly("CASH")
+        assertThat(service.getUser("user1")?.permissions).containsOnly("cash")
     }
 
     @Test

tools/explorer/src/main/kotlin/net/corda/explorer/views/NewTransaction.kt

@@ -22,7 +22,7 @@ import net.corda.node.services.messaging.CordaRPCOps
 import net.corda.node.services.messaging.startProtocol
 import net.corda.protocols.CashCommand
 import net.corda.protocols.CashProtocol
-import net.corda.protocols.TransactionBuildResult
+import net.corda.protocols.CashProtocolResult
 import org.controlsfx.dialog.ExceptionDialog
 import tornadofx.View
 import java.math.BigDecimal
@@ -144,12 +144,12 @@ class NewTransaction : View() {
                         rpcProxy.startProtocol(::CashProtocol, command).returnValue.toBlocking().first()
                     }.ui {
                         dialog.contentText = when (it) {
-                            is TransactionBuildResult.ProtocolStarted -> {
+                            is CashProtocolResult.Success -> {
                                 dialog.alertType = Alert.AlertType.INFORMATION
                                 dialog.setOnCloseRequest { resetScreen() }
                                 "Transaction Started \nTransaction ID : ${it.transaction?.id} \nMessage : ${it.message}"
                             }
-                            is TransactionBuildResult.Failed -> {
+                            is CashProtocolResult.Failed -> {
                                 dialog.alertType = Alert.AlertType.ERROR
                                 it.toString()
                             }