-
Couldn't load subscription status.
- Fork 44
Add daml models for token std based merging #2438
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Moritz Kiefer <[email protected]>
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.
Questions in comments...
| # Proxy contracts for delegating the usage of featured app rights | ||
|
|
||
| This is a convenience package that provides proxy contracts that allow for the | ||
| delegation of featured app rights to other parties for the usage of 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.
out of date
| do let transfer = choiceArg.transfer | ||
| require ("Sender in choice argument " <> show transfer.sender <> " matches owner " <> show owner) (transfer.sender == owner) | ||
| require ("Receiver in choice argument " <> show transfer.receiver <> " matches owner " <> show owner) (transfer.receiver == owner) | ||
| -- Note: This assumes you have preapprovals, if not you also need to accept the transfer instruction here. |
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.
Self transfers do not require pre-approvals... however did you want to do a combined send with merge?
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.
oh nice I totally missed that we have special handling for self-transfers. Excellent, will remove.
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.
however did you want to do a combined send with merge?
No I'd just do a merge only
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.
ack. I'll finish the review then. You'll probably nevertheless want to assert on the result being TransferResult_Completed
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.
You'll probably nevertheless want to assert on the result being TransferResult_Completed
added
Signed-off-by: Moritz Kiefer <[email protected]>
Signed-off-by: Moritz Kiefer <[email protected]>
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.
Thx. LGTM.
...l-delegation-test/daml/Splice/Util/MergeDelegation/IntegrationTests/TestMergeDelegation.daml
Outdated
Show resolved
Hide resolved
...l-delegation-test/daml/Splice/Util/MergeDelegation/IntegrationTests/TestMergeDelegation.daml
Outdated
Show resolved
Hide resolved
Co-authored-by: Simon Meier <[email protected]> Signed-off-by: moritzkiefer-da <[email protected]>
Signed-off-by: Moritz Kiefer <[email protected]>
Signed-off-by: Moritz Kiefer <[email protected]>
Signed-off-by: Moritz Kiefer <[email protected]>
| values = TextMap.fromList [("splice.lfdecentralizedtrust.org/reason", "merge")] | ||
| extraArgs = emptyExtraArgs | ||
|
|
||
| submit (actAs delegate <> readAs owner<> discloseMany (Map.values enrichedChoice.disclosures.disclosures)) $ exerciseCmd delegation MergeDelegation_Merge with |
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.
😲 great to know 💪
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines