Split the Issue, Move and Redeem flows into the general and an extra custom flow #47
Conversation
|
@illoyd, @kasiastreich, @roger3cev could you please share your thoughts about the approach. If it's the right way, then I'll merge new master into the branch and prepare cleaner pr. |
There was a problem hiding this comment.
Thanks for the PR, @IgorWolkov.
Firstly, great use of test driven development. Applying conditions to a token is a good real world scenario.
Secondly, this is a pluggable vs composable design question. In a pluggable design, hooks are added for developers to inject their own behaviours, while in a composable design developers are expected to build their own behaviours around existing building blocks. I believe the token team are keen on the composable model. For a basic example, please see EvolvableTokenHelpers and its use in CreateEvolvableToken and UpdateEvolvableToken.
To take this PR to the next level, I would recommend the following:
- Convert
Conditionsto an [encumbrance](https://docs.corda.r3.com/tutorial-contract.html#encumbrances}. This way you can apply conditions and constraints in a Corda way. - Create new issue, move, and redeem flows that assemble a full transaction, including tokens and commands, along with your new encumbrance. I do not believe the work has been completed on composable tokens yet, so this could be a good entry point to updating the code base - extract out the state and command composition from the existing flows for use in custom flows.
Check out Issue #19 for a conversation on pluggable vs composable.
@roger3cev, @kasiastreich, please step in if any of the above is not within your thinking.
Cheers, Ian
| import net.corda.core.flows.ReceiveFinalityFlow | ||
| import net.corda.core.flows.StartableByRPC | ||
| import net.corda.core.contracts.requireThat | ||
| import net.corda.core.flows.* |
| @@ -26,7 +26,7 @@ fun <T : TokenType> StartedMockNode.issueTokens( | |||
| issueTo: StartedMockNode, | |||
| notary: StartedMockNode, | |||
| amount: Amount<T>? = null, | |||
| anonymous: Boolean = true | |||
| anonymous: Boolean = false | |||
There was a problem hiding this comment.
Try not to change default values; other tests rely on them to be set a certain way and may impact test states and performance if changed.
| @@ -0,0 +1,70 @@ | |||
| package com.r3.corda.sdk.token.workflow | |||
| @@ -0,0 +1,56 @@ | |||
| package com.r3.corda.sdk.token.workflow.flows | |||
There was a problem hiding this comment.
Be careful with file names. This one has a typo.
| import javax.persistence.Table | ||
|
|
||
| @BelongsToContract(TradeConditionsContract::class) | ||
| data class TradeConditions(val owner: AbstractParty, |
There was a problem hiding this comment.
Instead of TradeConditions being a simple string, how about converting it to an encumbrance? This way you can model how an actual state constraint would work in Corda. (Of course, be aware of the caveats of not mixing an encumbrance with a reference state; this will work fine for tokens with a fixed (inline) definition but not for tokens using an EvolvableToken token type.)
Add just one space to trigger CI/CD build
One failed test
DiamondWithTokenScenarioTests, potential cause: anonymized parties handled in a wrong way.