search

corda / corda-settler

Corda Off-ledger settlement for all the things
Other
61 stars 36 forks source link

Added manual settlement rail #20

Closed countfloyd closed 5 years ago

countfloyd commented 5 years ago

This PR is a result of the work I did working Corda-settler into our Cordapp.

  1. The main change is the addition of a manual settlement rail. Since the oracle can't be used to verify manual payments, I added a flow to change a payment status. Also, made the settlementOracle field optional and the oracle will not be consulted if it's null

  2. Updated deprecated flow constructors to use 4.0 constructors

  3. Fixed some warnings

  4. Updated kittinunf.fuel to latest version

I've been testing quite extensively (except for the oracle, swift and ripple modules). Still need to add some unit test for the manual module. Hope this PR isn't too big :)

countfloyd commented 5 years ago

Not sure how I request another review. I'll just add a comment here.

aikkailim commented 5 years ago

Hi @countfloyd , may I ask what are you trying to do with this manual module? Is this a settlement rail you have created for yourself?

roger-that-dev commented 5 years ago

@yikailim93 I think manual settlement is quite useful if you need to settle something off ledger but the rail isn't yet supported. If you trust your counterpart, you can compromise some automation and assurance to handle settlements. Cheers

countfloyd commented 5 years ago

@yikailim93 Yes we created manual settlement because we need it for our client who do off ledger settlements by writing checks, etc. The obligee can manually verify the payment was received. At the moment, we have no way of automatically verifying a payment was made.

countfloyd commented 5 years ago

Thanks for addressing my comments - I added a couple of more for information (no need to action any of them). The only thing I'd ask is that is it possible for you to add a test for the manual settlement mechanism and some unit tests? Other than that, I think this is good to merge. Many thanks for the contribution!

Update: Forgot to mention that the other thing which needs adding is some contract code for cancelling an obligation. Cheers!

There's already contract verification code for cancelling an obligation, or did you mean something else?

aikkailim commented 5 years ago

Thank you both for the reply.

@yikailim93 Yes we created manual settlement because we need it for our client who do off ledger settlements by writing checks, etc. The obligee can manually verify the payment was received. At the moment, we have no way of automatically verifying a payment was made.

@countfloyd is your CorDapp currently up and running? And if I may ask, did you have to make a lot of changes to the cordapp/cordapp-contracts-states/oracle modules (or any other module) in order to integrate your manual settlement module? Also, are the source files in your manual settlement module the only/main files you have created to achieve your objective? Asking all these because I'm currently trying to develop my own settler as well but is facing a lot of roadblocks. Would appreciate if you could shed some light on this corda-settler.

roger-that-dev commented 5 years ago

There's already contract verification code for cancelling an obligation, or did you mean something else?

Sorry, my bad - I think I even added it. Doh!

countfloyd commented 5 years ago

@yikailim93 Yes our app is running in private test but not in production. All I really did was add my own ManualSettlement class and ManualPayment as well as a MakeManualPayment flow. I don't use the oracle at all because we have no way to automatically verifying that a payment was made. Instead, I made a UpdatePaymentStatusManually flow so that the obligee could verify that the payment was received. The manual module in Corda-settler was all I added to make obligations work for me, yes.

countfloyd commented 5 years ago

New changes for review:

  1. Added tests for manual module
  2. Fixed checkInvariantProperties in ObligationContract to throw exception. It was returning a Boolean that wasn't being used for anything.
  3. Added new checks in ObligationContract for handleUpdatePayment
  4. Couple of small changes suggested by @roger3cev
aikkailim commented 5 years ago

@yikailim93 Yes our app is running in private test but not in production. All I really did was add my own ManualSettlement class and ManualPayment as well as a MakeManualPayment flow. I don't use the oracle at all because we have no way to automatically verifying that a payment was made. Instead, I made a UpdatePaymentStatusManually flow so that the obligee could verify that the payment was received. The manual module in Corda-settler was all I added to make obligations work for me, yes.

Thank you @countfloyd! This information is very helpful. May I ask if you can share the commands you ran on the nodes' terminal for the process from creating an obligation up until settlement? This will solely be for demonstration purposes.

Also, if I may add on, I was trying to run the corda-settler repo that you have uploaded on your profile. I encountered the error below when running gradlew clean deployNodes, do you have any idea what went wrong?

C:\Users\aikka\Desktop\corda-settler-manual>gradlew --stacktrace clean deployNodes
> Task :cordapp-contracts-states:clean FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':cordapp-contracts-states:clean'.
> Unable to delete file: C:\Users\aikka\Desktop\corda-settler-manual\cordapp-contracts-states\build\libs\cordapp-contracts-states-0.1.jar

* Try:
Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Exception is:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':cordapp-contracts-states:clean'.
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:110)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:77)
        at org.gradle.api.internal.tasks.execution.OutputDirectoryCreatingTaskExecuter.execute(OutputDirectoryCreatingTaskExecuter.java:51)
        at org.gradle.api.internal.tasks.execution.SkipUpToDateTaskExecuter.execute(SkipUpToDateTaskExecuter.java:59)
        at org.gradle.api.internal.tasks.execution.ResolveTaskOutputCachingStateExecuter.execute(ResolveTaskOutputCachingStateExecuter.java:54)
        at org.gradle.api.internal.tasks.execution.ValidatingTaskExecuter.execute(ValidatingTaskExecuter.java:59)
        at org.gradle.api.internal.tasks.execution.SkipEmptySourceFilesTaskExecuter.execute(SkipEmptySourceFilesTaskExecuter.java:101)
        at org.gradle.api.internal.tasks.execution.FinalizeInputFilePropertiesTaskExecuter.execute(FinalizeInputFilePropertiesTaskExecuter.java:44)
        at org.gradle.api.internal.tasks.execution.CleanupStaleOutputsExecuter.execute(CleanupStaleOutputsExecuter.java:91)
        at org.gradle.api.internal.tasks.execution.ResolveTaskArtifactStateTaskExecuter.execute(ResolveTaskArtifactStateTaskExecuter.java:62)
        at org.gradle.api.internal.tasks.execution.SkipTaskWithNoActionsExecuter.execute(SkipTaskWithNoActionsExecuter.java:59)
        at org.gradle.api.internal.tasks.execution.SkipOnlyIfTaskExecuter.execute(SkipOnlyIfTaskExecuter.java:54)
        at org.gradle.api.internal.tasks.execution.ExecuteAtMostOnceTaskExecuter.execute(ExecuteAtMostOnceTaskExecuter.java:43)
        at org.gradle.api.internal.tasks.execution.CatchExceptionTaskExecuter.execute(CatchExceptionTaskExecuter.java:34)
        at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.run(EventFiringTaskExecuter.java:51)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:301)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:293)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:175)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:91)
        at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
        at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter.execute(EventFiringTaskExecuter.java:46)
        at org.gradle.execution.taskgraph.LocalTaskInfoExecutor.execute(LocalTaskInfoExecutor.java:42)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareWorkItemExecutor.execute(DefaultTaskExecutionGraph.java:277)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareWorkItemExecutor.execute(DefaultTaskExecutionGraph.java:262)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$ExecutorWorker$1.execute(DefaultTaskPlanExecutor.java:135)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$ExecutorWorker$1.execute(DefaultTaskPlanExecutor.java:130)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$ExecutorWorker.execute(DefaultTaskPlanExecutor.java:200)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$ExecutorWorker.executeWithWork(DefaultTaskPlanExecutor.java:191)
        at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$ExecutorWorker.run(DefaultTaskPlanExecutor.java:130)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:46)
        at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
Caused by: org.gradle.api.file.UnableToDeleteFileException: Unable to delete file: C:\Users\aikka\Desktop\corda-settler-manual\cordapp-contracts-states\build\libs\cordapp-contracts-states-0.1.jar
        at org.gradle.api.internal.file.delete.Deleter.handleFailedDelete(Deleter.java:109)
        at org.gradle.api.internal.file.delete.Deleter.doDeleteInternal(Deleter.java:86)
        at org.gradle.api.internal.file.delete.Deleter.doDeleteInternal(Deleter.java:81)
        at org.gradle.api.internal.file.delete.Deleter.doDeleteInternal(Deleter.java:81)
        at org.gradle.api.internal.file.delete.Deleter.delete(Deleter.java:66)
        at org.gradle.api.tasks.Delete.clean(Delete.java:67)
        at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:73)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.doExecute(StandardTaskAction.java:46)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:39)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:26)
        at org.gradle.api.internal.AbstractTask$TaskActionWrapper.execute(AbstractTask.java:801)
        at org.gradle.api.internal.AbstractTask$TaskActionWrapper.execute(AbstractTask.java:768)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter$1.run(ExecuteActionsTaskExecuter.java:131)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:301)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:293)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:175)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:91)
        at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeAction(ExecuteActionsTaskExecuter.java:120)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:99)
        ... 31 more

* Get more help at https://help.gradle.org

BUILD FAILED in 2s
3 actionable tasks: 3 executed

PS: I'm really sorry to keep bothering you here, I've been trying to develop my own settler for the past 2 months but kept getting errors and I'm still no where near completing my cordapp. Really hope that you wouldn't mind me asking all the questions.

roger-that-dev commented 5 years ago

@countfloyd I've tried to run the tests locally but none of them pass:

1) The gradle plugins version was updated but we need to disable sealing for all the cordapps because it doesn't work with the way the packages are currently structured.

cordapp { sealing { enabled false } }

2) it seems the create obligation flow doesn't work - sends and receives are not matched up. Need to add subFlow(SwapIdentitiesFlow(otherFlow)) in the responder flow as the first sub flow, to match up the sends and receives. The new swap identities flow in Corda 4 is not an initiating flow, so you need to add the responder flow.

3) Also, the FinalityFlow in SendToSettlementOracle needs a responder. This will be more tricky than you think to implement though.

Cheers

roger-that-dev commented 5 years ago

@yikailim93 can you please stop asking support questions in this PR? Either go on to slack and ask a question, use stack over flow or submit a github issue. Me or others will be happy to help you in those places. Cheers

aikkailim commented 5 years ago

@roger3cev okay, sorry for the inconvenience caused.

roger-that-dev commented 5 years ago

@yikailim93 no worries - is also better for you to ask questions in those places. There's more people to help :)

countfloyd commented 5 years ago

@roger3cev Hmmm weird. The tests passed for me. Although I only ran them from my IDE. I'll check it out along with your other issues. Won't be able to work on it until next week. Thanks for your review.

countfloyd commented 5 years ago

@roger3cev, addressed the issues you outlined. Just wondering what I need to run the SWIFT unit tests. README says I need to email you for API key and I need a swift.pem file? Can you email me what I need?

XRP tests all run successfully now.

Thanks.

roger-that-dev commented 5 years ago

@countfloyd dont worry about the SWIFT tests - I'll sort those out. Cheers!

roger-that-dev commented 5 years ago

This looks great - good to merge!

roger-that-dev commented 5 years ago

Thanks for the contribution @countfloyd !

mikehearn commented 5 years ago

Yes, thanks @countfloyd - great work and much appreciated by all.

countfloyd commented 5 years ago

My pleasure, thanks for the work reviewing this.