Skip to content
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

Core-16349: corda 5 Kotlin negotiation app #21

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

7Dhruv
Copy link
Contributor

@7Dhruv 7Dhruv commented Aug 24, 2023

No description provided.

// The CorDapp uses the slf4j logging framework. Corda-API provides this so we need a 'cordaProvided' declaration.
cordaProvided 'org.slf4j:slf4j-api'

// This are shared so should be here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block should probably be deleted

// The CorDapp uses the slf4j logging framework. Corda-API provides this so we need a 'cordaProvided' declaration.
cordaProvided 'org.slf4j:slf4j-api'

// Dependencies Required By Simulator Tests

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to remove

Copy link
Collaborator

@peterli-r3 peterli-r3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Umm, where are the gradle files, and config file for deployment?

@7Dhruv 7Dhruv requested a review from a team as a code owner August 25, 2023 08:16
@7Dhruv 7Dhruv requested a review from peterli-r3 August 25, 2023 08:29
@7Dhruv
Copy link
Contributor Author

7Dhruv commented Aug 25, 2023

Umm, where are the gradle files, and config file for deployment?

  • Included them in the new commit

@7Dhruv 7Dhruv requested a review from tlawson3 August 25, 2023 08:31
@peterli-r3
Copy link
Collaborator

image
Something is not right. You have long hanging flow. All the function works, but it seems there is continuous running flows.

@7Dhruv
Copy link
Contributor Author

7Dhruv commented Aug 29, 2023

image Something is not right. You have long hanging flow. All the function works, but it seems there is continuous running flows.

Do you mean that the flow is taking too long to execute? and does this happen in java version of this app as well or is it just this one?

var flowEngine: FlowEngine? = null

@Suspendable
override fun call(requestBody: ClientRequestBody): String {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does ProposalFlow return a string? Is it not supposed to return a UUID? Taking into account that Accept and Modify flows seem to be expecting the proposal ID to be a UUID

- updated accept contract verification
- Resolved hanging flows bug
- updated readme.md
"clientRequestId": "AcceptFlow",
"flowClassName": "com.r3.developers.samples.negotiation.workflows.Accept.AcceptFlowRequest",
"requestBody": {
"proposalID": "<use the proposal id here>"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing the variable acceptor, but why do you need this input?
The accepter can only be the proposee of the proposal. In the contract, you can just use the proposee variable.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @peterli-r3 I've added the Acceptor to the documentation. We still need to know who is trying to accept the trade to be able to perform the necessary checks in the Accept contract.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @khutsijabari , I had quickly chatted with @7Dhruv eariler, You dont really need to add a variable to keep track of the proposer. You can check it in two ways:

  1. Check if the proposer with myInfo in the AcceptProposal flow initiator.
  2. Make the accept command take input, we can pass in the myInfo into the contract via the command.
    The idea is that we will try to have as less variables as possible in the states.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants