-
Notifications
You must be signed in to change notification settings - Fork 45
Solana DvP sample #123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Solana DvP sample #123
Conversation
…ousetoken-atomic-dvp
…ousetoken-atomic-dvp
Solana/delivery-vs-payment/README.md
Outdated
| The CordApp configuration file contains the following Solana account setting for a participant (Corda party): | ||
|
|
||
| ``solanaWalletFile`` - the file path with public-private key pair of the participant's wallet | ||
| ``solanaTokenMint`` - public key of the mint account of SPL token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This config needs to be stablecoinTokenMint, and the description updated
Solana/delivery-vs-payment/README.md
Outdated
|
|
||
| ``solanaWalletFile`` - the file path with public-private key pair of the participant's wallet | ||
| ``solanaTokenMint`` - public key of the mint account of SPL token | ||
| ``solanaRpcUrl`` - Solana RPC URL to obtain account info; e.g.: ``http://127.0.0.1:8899`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URL of the RPC provider for interacting with the blockchain. If you are using the test validator then this will be http://127.0.0.1:8899. If you want to use devnet then the URL is https://api.devnet.solana.com.
Solana/delivery-vs-payment/README.md
Outdated
| ``solanaWalletFile`` - the file path with public-private key pair of the participant's wallet | ||
| ``solanaTokenMint`` - public key of the mint account of SPL token | ||
| ``solanaRpcUrl`` - Solana RPC URL to obtain account info; e.g.: ``http://127.0.0.1:8899`` | ||
| ``solanaWsUrl`` - Solana WS URL, not used by a flow in this sample, however needed by underlying client, for future use; e.g.: ``ws://127.0.0.1:8900`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The corresponding websocket URL of the RPC provider. ws://127.0.0.1:8900 for the test validator and wss://api.devnet.solana.com for devnet.
(There's no need to mention the detail that it's not used directly).
Solana/delivery-vs-payment/README.md
Outdated
| ``solanaRpcUrl`` - Solana RPC URL to obtain account info; e.g.: ``http://127.0.0.1:8899`` | ||
| ``solanaWsUrl`` - Solana WS URL, not used by a flow in this sample, however needed by underlying client, for future use; e.g.: ``ws://127.0.0.1:8900`` | ||
|
|
||
| Public keys are written in Base58 format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which public keys are written?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Public keys are written in Base58 format." is a left over, deleting it
Solana/delivery-vs-payment/README.md
Outdated
| ### Resiliency | ||
|
|
||
| TODO No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a sample, so no need to be open this can of worms.
Solana/delivery-vs-payment/README.md
Outdated
|
|
||
| Public keys are written in Base58 format. | ||
|
|
||
| ## Pre-Requisites |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it makes more sense for the pre-requisites to not only be before the config section, but be right at the beginning of the README. Also, the config section should be after that, before the concepts and details section. You have the first paragraph as a quick intro, then it should explain how to run the sample. And then it shoudl have the details of what the sample does.
Solana/delivery-vs-payment/README.md
Outdated
|
|
||
| Public keys are written in Base58 format. | ||
|
|
||
| ## Pre-Requisites |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section also needs link to installing Solana
| corda_release_group = constants.getProperty("cordaReleaseGroup") | ||
| corda_core_release_group = constants.getProperty("cordaCoreReleaseGroup") | ||
| corda_release_version = constants.getProperty("cordaVersion") | ||
| corda_core_release_version = constants.getProperty("cordaCoreVersion") | ||
| corda_gradle_plugins_version = constants.getProperty("gradlePluginsVersion") | ||
| corda_solana_notary_version = constants.getProperty("cordaSolanaNotaryVersion") | ||
| corda_tokens_release_version = constants.getProperty("cordaTokensVersion") | ||
| sava_version = constants.getProperty("savaVersion") | ||
| corda_platform_version = constants.getProperty("platformVersion").toInteger() | ||
| kotlin_version = constants.getProperty("kotlinVersion") | ||
| junit_version = constants.getProperty("junitVersion") | ||
| log4j_version = constants.getProperty("log4jVersion") | ||
| slf4j_version = constants.getProperty("slf4jVersion") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we're followng existing samples, but this "pattern" is really stupid. I don't know why we ended up doing this in Corda/enterprise. If these constants are just put into the grade.properties file then they're automatically available in the build.gradle files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the constants.properties file is one level above the project folder, and it's shared by all modules (here will be this sample and the bridging sample), we could remove this constants file with properties duplication for each module
| log4j_version = constants.getProperty("log4jVersion") | ||
| slf4j_version = constants.getProperty("slf4jVersion") | ||
|
|
||
| testJvmArgs = ['--add-opens', 'java.base/java.time=ALL-UNNAMED', '--add-opens', 'java.base/java.io=ALL-UNNAMED', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be no need to have to define this list separately and then use in multiple places. The tasks.withType(Test) block below is already setting this to all test tasks, so this should be inlined there and the other places it's used removed.
| "Stock symbol must not be empty".using(!createdStockState.symbol.isEmpty()) | ||
| "Stock name must not be empty".using(!createdStockState.name.isEmpty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't these be in the init block of StockState?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep this identical what is in stockbuydivided code, I presume the original code either didn't take init into an account, or it wanted to make it more explicit/enrich contract logic. The benefit of the latter is that you don't need to know about init semantics, and all business "checks" are in verify function. But I can change it here and move to init.
|
|
||
| @Test | ||
| fun `Agree verifies with correct output, signers, and Solana instruction`() { | ||
| val out = sampleState() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
output
Solana/delivery-vs-payment/README.md
Outdated
| @@ -0,0 +1,68 @@ | |||
| # Solana DvP Sample CorDapp | |||
|
|
|||
| This CorDapp provides an example to perform Delivery vs Payment ("DvP") transaction of an asset (a stock shares) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add "atomic" in there
Solana/constants.properties
Outdated
| cordaEnterpriseReleaseGroup=com.r3.corda | ||
| cordaOsReleaseGroup=net.corda | ||
| cordaEnterpriseVersion=4.14-20251230.004000-52 | ||
| cordaOsVersion=4.14-SNAPSHOT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we pin this down as well?
| Corda notary requires additional settings in the node configuration (``node.conf`` file). | ||
| They are grouped under ``solana`` sub-entry of ``notarty``: | ||
|
|
||
| - ``rpcUrl`` URL of the RPC provider for interacting with the blockchain. If you are using the test validator | ||
| then this will be `http://127.0.0.1:8899`; if you want to use devnet then the URL is `https://api.devnet.solana.com` | ||
| - ``websocketUrl`` The corresponding websocket URL of the RPC provider. `ws://127.0.0.1:8900` for the test validator | ||
| and `wss://api.devnet.solana.com` for devnet | ||
| - ``notaryKeypairFile`` The notary [file-system wallet](https://docs.solanalabs.com/cli/wallets/file-system) | ||
| for singing Corda Program on Solana | ||
| - ``custodiedKeysDir`` The directory for notary to store Corda participant Solana file-system wallets | ||
| for singing stablecoin transactions, these should be located in a different directory than the notary wallet | ||
| - ``programWhitelist`` the list of Solana Programs that can be run by the Notary, set to address of SPL Token Program |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to move this to the docsite as the general solana notary config needs to be documented
| @Test | ||
| fun `Agree verifies with correct output, signers, and Solana instruction`() { | ||
| val out = sampleState() | ||
| val outpput = sampleState() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
| "No inputs should be consumed." using (tx.inputsOfType<SharesPaymentState>().isEmpty()) | ||
| val outputs = tx.outputsOfType<SharesPaymentState>() | ||
| "One output should be created." using (outputs.size == 1) | ||
| val output = tx.outputsOfType<SharesPaymentState>().single() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor:singleOrNull and then requireNotNull
mkitR3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I think the current implementation is sufficient for the demo purposes.
New CorDapp provides an example to perform a DvP (Delivery vs Payment) with Payment on Solana.
For details see README.md file.