PLT-8891: Add marlowe-object contract flow example #136
Conversation
WalkthroughRecent updates introduce a Node.js example showcasing the creation and interaction with Marlowe contracts using the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to filter (5)
- examples/nodejs/package-lock.json
- examples/nodejs/package.json
- examples/nodejs/src/tsconfig.json
- examples/nodejs/tsconfig.json
- tsconfig.json
Files selected for processing (6)
- examples/nodejs/src/marlowe-object-flow.ts (1 hunks)
- examples/nodejs/src/wallet-flow.ts (1 hunks)
- packages/language/core/v1/src/contract.ts (1 hunks)
- packages/language/core/v1/src/index.ts (1 hunks)
- packages/runtime/lifecycle/src/api.ts (5 hunks)
- packages/runtime/lifecycle/src/generic/runtime.ts (1 hunks)
Additional comments: 5
packages/language/core/v1/src/contract.ts (1)
- 334-365: The implementation of
minBigintandgetNextTimeoutfunctions appears to be correct. ThegetNextTimeoutfunction uses pattern matching to handle different contract types and seems to handle thewhencontract case appropriately by checking ifminTimeis greater thanwhenContract.timeout. If it is, the function recursively calls itself with thetimeout_continuationof thewhencontract; otherwise, it returns thetimeoutof thewhencontract. This logic is consistent with the expected behavior of finding the next timeout after a given time.packages/language/core/v1/src/index.ts (1)
- 43-43: The addition of
getNextTimeoutto the exports inindex.tsaligns with the PR's objective to expose this function as part of the public API.packages/runtime/lifecycle/src/api.ts (2)
13-13: The import statement has been correctly updated to include both
RestClientandRestDI.26-26: The
RuntimeLifecycletype has been correctly updated to include the newrestClientproperty.packages/runtime/lifecycle/src/generic/runtime.ts (1)
- 16-16: The addition of the
restClientproperty to theRuntimeLifecyclereturn type is consistent with the PR's objectives. Ensure that the new property is properly integrated and utilized in the codebase whereRuntimeLifecycleobjects are used.
| ): RuntimeLifecycle { | ||
| return { | ||
| wallet: wallet, | ||
| restClient, | ||
| contracts: mkContractLifecycle(wallet, deprecatedRestAPI, restClient), | ||
| payouts: mkPayoutLifecycle(wallet, deprecatedRestAPI, restClient), | ||
| }; |
There was a problem hiding this comment.
💡 NOTE
Codebase verification is a beta feature.
The verification process has identified several issues with the current invocations of the mkRuntimeLifecycle function:
- In
packages/runtime/lifecycle/test/provisionning.ts, both the higher-order function on line 75 and the direct invocation on line 99 need to be updated to include therestClientparameter. - In
examples/nodejs/src/marlowe-object-flow.tsandexamples/nodejs/src/escrow-flow.ts, the calls tomkRuntimeLifecycleare incorrect as they pass an object instead of individual parameters. These calls need to be updated to match the new function signature that includes therestClientparameter.
Analysis chain
The mkRuntimeLifecycle function now requires an additional restClient parameter. Verify that all invocations of this function across the codebase have been updated to include this new argument.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for `mkRuntimeLifecycle` function calls without the `restClient` parameter.
ast-grep --lang typescript --pattern $'mkRuntimeLifecycle($_, $_, $_)'
# Search for `mkRuntimeLifecycle` function calls with the `restClient` parameter to ensure they are updated.
ast-grep --lang typescript --pattern $'mkRuntimeLifecycle($_, $_, $_, $_)'
Length of output: 147
Script:
#!/bin/bash
# Search for any calls to `mkRuntimeLifecycle` to ensure all possible invocations are caught.
ast-grep --lang typescript --pattern $'mkRuntimeLifecycle($$$)'
Length of output: 797
Script:
#!/bin/bash
# Check the context around the higher-order function in `provisionning.ts` to understand how it's used with `mkRuntimeLifecycle`.
rg --context 5 "mkRuntimeLifecycle" "packages/runtime/lifecycle/test/provisionning.ts"
Length of output: 818
hrajchert
force-pushed
the
plt-8891-add-merkleized-contract-flow-example
branch
3 times, most recently
from
6872a25 to
921cd84
Compare
hrajchert
force-pushed
the
plt-8891-add-merkleized-contract-flow-example
branch
3 times, most recently
from
936fbf1 to
de3da3f
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files ignored due to filter (8)
- changelog.d/scriv.ini
- examples/nodejs/package-lock.json
- examples/nodejs/package.json
- examples/nodejs/src/tsconfig.json
- examples/nodejs/tsconfig.json
- packages/adapter/package.json
- tsconfig-base.json
- tsconfig.json
Files selected for processing (24)
- .vscode/settings.json (1 hunks)
- changelog.d/20240108_165452_hrajchert_plt_8891_add_merkleized_contract_flow_example.md (1 hunks)
- examples/nodejs/.gitignore (1 hunks)
- examples/nodejs/src/config.ts (1 hunks)
- examples/nodejs/src/experimental-features/applicable-inputs.ts (1 hunks)
- examples/nodejs/src/experimental-features/metadata.ts (1 hunks)
- examples/nodejs/src/marlowe-object-flow.ts (1 hunks)
- examples/nodejs/src/wallet-flow.ts (1 hunks)
- jsdelivr-npm-importmap.js (1 hunks)
- packages/adapter/src/bigint.ts (1 hunks)
- packages/adapter/src/time.ts (1 hunks)
- packages/language/core/v1/src/contract.ts (2 hunks)
- packages/language/core/v1/src/index.ts (1 hunks)
- packages/language/core/v1/src/inputs.ts (3 hunks)
- packages/language/core/v1/src/semantics.ts (11 hunks)
- packages/language/core/v1/src/transaction.ts (2 hunks)
- packages/marlowe-object/src/guards.ts (1 hunks)
- packages/marlowe-object/src/index.ts (1 hunks)
- packages/marlowe-object/src/object.ts (1 hunks)
- packages/runtime/client/rest/src/index.ts (3 hunks)
- packages/runtime/client/rest/src/pagination.ts (2 hunks)
- packages/runtime/lifecycle/src/api.ts (6 hunks)
- packages/runtime/lifecycle/src/generic/contracts.ts (4 hunks)
- packages/runtime/lifecycle/src/generic/runtime.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- .vscode/settings.json
- examples/nodejs/.gitignore
- packages/marlowe-object/src/guards.ts
Additional comments: 52
packages/adapter/src/bigint.ts (1)
- 5-6: The implementation of
minandmaxfunctions forbiginttypes looks good and follows best practices for utility functions.examples/nodejs/src/experimental-features/metadata.ts (1)
- 3-8: The
splitAddressfunction is implemented correctly and follows good practices in terms of simplicity and readability.packages/runtime/lifecycle/src/generic/runtime.ts (1)
- 16-16: The addition of the
restClientparameter to theRuntimeLifecycleobject is correctly implemented.packages/adapter/src/time.ts (1)
- 16-17: The
iso8601ToPosixTimefunction is correctly implemented and follows best practices for time conversion functions.examples/nodejs/src/config.ts (1)
- 22-23: The
readConfigfunction is correctly updated to accept an optional path parameter, enhancing its flexibility.packages/runtime/client/rest/src/pagination.ts (1)
- 20-20: Making the
currentproperty optional in thePageinterface is a reasonable change that can accommodate pagination responses without a current page.packages/marlowe-object/src/index.ts (1)
- 53-53: The export of the
ContractBundleis correctly added to the list of exported entities.examples/nodejs/src/wallet-flow.ts (1)
- 52-54: The addition of error handling to the
mainfunction with a catch block is a good practice to ensure any unhandled errors are logged.changelog.d/20240108_165452_hrajchert_plt_8891_add_merkleized_contract_flow_example.md (1)
- 4-35: The changelog entries are well-documented and provide a clear overview of the features and changes introduced in the PR.
packages/language/core/v1/src/index.ts (1)
- 66-66: The export of the
getNextTimeoutfunction is correctly added to the list of exported entities.packages/marlowe-object/src/object.ts (1)
- 175-186: The
ContractBundleinterface and its correspondingContractBundleGuardare correctly defined and follow best practices for type safety.packages/language/core/v1/src/inputs.ts (1)
- 119-160: The new interfaces and types for Merkleized inputs are correctly added, and the
MerkleizedInputtype is appropriately updated to be a union of these new types.jsdelivr-npm-importmap.js (1)
- 7-8: The addition of the
@marlowe.io/adapter/bigintentry to the import map is consistent with the existing pattern and versioning. Looks good.packages/language/core/v1/src/transaction.ts (2)
42-42: The updated comment for
PaymentGuardadds clarity on the composition of a marlowe transaction with multiple inputs within a time interval. This is a helpful clarification.59-72: The addition of the
SingleInputTxinterface is well-documented and follows TypeScript best practices for interface declarations. The comments provide useful insights into the use of the interface.packages/language/core/v1/src/contract.ts (2)
16-16: The import of
Bigfrom@marlowe.io/adapter/bigintis correctly implemented and necessary for thegetNextTimeoutfunction that follows.335-363: The
getNextTimeoutfunction is a well-documented and cleanly implemented addition that uses pattern matching to handle different contract types and determine the next timeout. It correctly utilizes theBig.minfunction from the@marlowe.io/adapter/bigintmodule to compare timeouts.packages/runtime/lifecycle/src/generic/contracts.ts (2)
3-3: The import of
Optionfrom "fp-ts/lib/Option.js" is added to handle optional values in a functional programming style. This is a standard practice in FP and should be fine as long as it's used consistently throughout the codebase.22-22: The import of
BlockHeaderfrom "@marlowe.io/runtime-core" is added, presumably for use in the newgetInputHistoryfunction to sort transactions based on block headers.packages/runtime/lifecycle/src/api.ts (4)
13-13: The import statement now includes
RestClient, which is used in theRuntimeLifecycletype. This change aligns with the addition ofrestClientas a dependency for various lifecycle functions.27-27: The
RuntimeLifecycletype has been modified to includerestClient: RestClient. This change is consistent with the PR's objective to integrate theRestClientinto the lifecycle management.23-23: The import of
SingleInputTxfrom "@marlowe.io/language-core-v1/transaction.js" is added, which is likely used in the newly addedgetInputHistorymethod in theContractsAPIinterface.273-273: The
getInputHistorymethod is added to theContractsAPIinterface, which aligns with the implementation provided in thecontracts.tsfile. This method is part of the new functionality to retrieve the transaction history for a contract.examples/nodejs/src/experimental-features/applicable-inputs.ts (5)
48-73: The
AppliedActionResultinterface is well-documented and clearly defines the expected results of applying an action. Ensure that the documentation remains up-to-date if any changes are made to the interface in the future.76-79: The
CanNotifyinterface defines atypeand apolicyIdalong with anapplyActionmethod. It's important to ensure that theapplyActionmethod is implemented correctly in all classes that implement this interface.122-195: The
getApplicableActionsfunction is complex and performs several asynchronous operations. It's crucial to ensure that error handling is robust, especially since the function interacts with external services viarestClient. Additionally, the function should be tested thoroughly to handle various edge cases and potential failures in external service communication.Verification successful
Given the absence of a try-catch block within the
getApplicableActionsfunction, as indicated by the ast-grep script output, it appears that the function may not have robust error handling implemented in the traditional sense. However, this does not necessarily mean that error handling is not present in another form. To provide a more comprehensive verification, we should investigate other error handling mechanisms that may be in place.
The script contained an error due to the misuse of the
--execflag withast-grep, which does not support this flag. To proceed, we will directly usergto search for patterns of error handling within the file containing thegetApplicableActionsfunction.
The script has successfully identified several instances where
throw new Erroris used within thegetApplicableActionsfunction in theapplicable-inputs.tsfile. This indicates that the function does implement some form of error handling by throwing exceptions when certain error conditions are met.However, the absence of
catchblocks or.catch()methods suggests that while the function can throw errors, it may not be handling them internally. Instead, it could be relying on the caller to handle these exceptions.Based on the script output, it seems that the function is designed to propagate errors up the call stack rather than handling them internally. This is a valid error handling strategy, especially in asynchronous functions that are expected to be used with
await, where the caller can use try-catch blocks to handle the errors.* 198-224: The `mkPartyFilter` function creates a filter based on the wallet's addresses and tokens. It's important to ensure that the filter logic is correct and that it handles all possible cases for parties and policy IDs. Additionally, consider the performance implications if the tokens array is large.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that error handling is robust in getApplicableActions function. ast-grep --lang typescript --pattern $'async function getApplicableActions($_, $_, $_) { try { $$$ } catch ($_) { $$$ } }'Length of output: 137
Script:
#!/bin/bash # Check for promise rejection handling with .catch() or async/await with try-catch in the getApplicableActions function. ast-grep --lang typescript --pattern $'async function getApplicableActions($_, $_, $_) { $$$ }' --exec 'rg "catch\(|throw new Error\("'Length of output: 332
Script:
#!/bin/bash # Find the file containing getApplicableActions and check for error handling patterns. file_path=$(fd applicable-inputs.ts) rg "catch\(|throw new Error\(" $file_pathLength of output: 540
- 322-346: The
mergeBoundsfunction is used to merge overlapping bounds. It's important to ensure that the sorting and merging logic is correct and that it handles all edge cases, such as adjacent bounds that should be merged.Verification successful
The
mergeBoundsfunction exists in the fileexamples/nodejs/src/experimental-features/applicable-inputs.ts. To verify the logic of the function, we need to examine its implementation details. Since the function is present, we can proceed to check the sorting and merging logic within the function to ensure it handles overlapping and adjacent bounds correctly.
The
mergeBoundsfunction inexamples/nodejs/src/experimental-features/applicable-inputs.tssorts the bounds and merges them if they overlap or are adjacent. The logic checks if thefromvalue of the current bound is less than or equal to thetovalue of the previous bound to decide if a merge is necessary. If so, it updates thetovalue to the maximum of the twotovalues. If not, it pushes the current bound to themergedBoundsarray and starts a new current bound. The function appears to handle both overlapping and adjacent bounds as intended.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that mergeBounds function correctly merges overlapping and adjacent bounds. ast-grep --lang typescript --pattern $'function mergeBounds($_) { $$$ }'Length of output: 75
Script:
#!/bin/bash # Search for the definition of the mergeBounds function to verify its logic. ast-grep --lang typescript --pattern $'function mergeBounds($$$) { $$$ }'Length of output: 76
Script:
#!/bin/bash # Use ripgrep to search for the mergeBounds function in the codebase. rg 'function mergeBounds'Length of output: 139
Script:
#!/bin/bash # Extract the implementation details of the mergeBounds function to verify its logic. rg -A 20 'function mergeBounds' examples/nodejs/src/experimental-features/applicable-inputs.tsLength of output: 689
packages/runtime/client/rest/src/index.ts (2)
18-18: The import statement has been updated to use
ContractBundleinstead ofLabelandBundle. This change aligns with the PR's objective to work with marlowe-object contracts.90-93: The
createContractSourcesfunction now takes aContractBundleobject as a parameter. Ensure that all calls to this function have been updated to pass the correct object structure.examples/nodejs/src/marlowe-object-flow.ts (14)
38-38: The previous comment regarding the placement of the
main()function call is still valid. It should be moved to the bottom of the file or wrapped in a guard to prevent it from running on module import.41-64: The
parseClifunction correctly parses command-line arguments and handles the--helpoption. It is well-structured and follows best practices.74-91: The
waitIndicatorfunction is implemented correctly and provides clear feedback to the user. It uses a non-blocking loop to write dots to the console, which is a good practice for CLI tools.97-119: The validators for bech32 addresses and positive bigints are implemented correctly and provide clear error messages. They follow best practices for input validation.
126-134: The
dateInFutureValidatorfunction correctly checks if the provided date is in the future. It is a good practice to validate user input, especially when dealing with dates.142-200: The
createContractMenufunction is well-structured and uses the validators correctly. It also provides clear feedback to the user about the contract creation process.207-236: The
loadContractMenufunction correctly handles the loading of an existing contract. It validates the contract's tags and provides appropriate feedback if the contract is invalid.241-315: The
contractMenufunction is well-implemented, with clear logic for handling different contract actions. It uses thegetApplicableActionsandmkApplicableActionsFilterfunctions to determine the actions available to the user.318-351: The
mainLoopfunction is correctly implemented and handles the main menu of the CLI tool. It provides a clear structure for the different actions a user can take.359-380: The
DelayPaymentSchemeinterface and related types are well-defined and provide a clear structure for the delay payment contract parameters.383-418: The
mkDelayPaymentfunction correctly constructs a contract bundle for delay payment contracts. It uses thedatetoTimeoutfunction to convert dates to timeouts, which is appropriate for the contract logic.493-519: The
getStatefunction correctly determines the contract's state based on the transaction history and current time. It handles different scenarios and provides appropriate error handling.605-643: The
validateExistingContractfunction correctly checks if the contract ID corresponds to a delay payment contract. It uses deep equality to compare the initial contract with the expected contract, which is a robust method for validation.645-666: The
mainfunction is well-structured and sets up the necessary configurations for the CLI tool. It initializes the wallet and runtime lifecycle correctly and starts the main loop.packages/language/core/v1/src/semantics.ts (8)
62-62: The import statement for
IChoiceandIDepositfrominputs.jshas been added. This change is likely related to the new functionality introduced in the PR for handling different types of inputs.90-90: The import of the
Bigmodule from@marlowe.io/adapter/bigintis new. This change is consistent with the replacement ofminBigintandmaxBigintfunctions withBig.minandBig.maxmethods later in the file.95-95: The export of
SingleInputTxis new. Ensure that this export is being used elsewhere in the codebase and that it is necessary to be public.Verification successful
The export of
SingleInputTxis being used in multiple files across the codebase, including lifecycle API and contract files, as well as in an example Node.js application. This confirms that the export is necessary and is being utilized as intended.* 433-433: The use of `Big.min` to determine the `paidAmount` is a change from the previous `minBigint` function. This is a correctness improvement if the `Big` library handles `bigint` operations more accurately or efficiently.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of SingleInputTx across the codebase. rg --type ts "SingleInputTx"Length of output: 932
535-543: The addition of the
ContractQuiescentReduceResulttype is new. This type seems to be part of the new functionality for handling contract reduction results. Ensure that this new type is integrated correctly throughout the codebase.790-790: The
convertReduceWarningfunction is exported, which is a new addition. Verify that this function is used elsewhere in the codebase and that its export is necessary.Verification successful
The
convertReduceWarningfunction is used within thesemantics.tsfile and is also referenced in theapplicable-inputs.tsfile within the Node.js examples. This confirms that the function's export is justified as it is utilized in other parts of the codebase.* 841-841: The `applyAllInputs` function is exported, which is new. As with other exports, ensure that this function is used elsewhere and that its export is necessary.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of convertReduceWarning across the codebase. rg --type ts "convertReduceWarning"Length of output: 602
Verification successful
The
applyAllInputsfunction is used both internally withinsemantics.tsand externally in theexamples/nodejs/src/experimental-features/applicable-inputs.tsfile. This confirms that the function's export is necessary for its usage outside of its definition module.* 929-929: The use of `Big.max` to determine the `newFrom` time is a change from the previous `maxBigint` function. This is a correctness improvement if the `Big` library handles `bigint` operations more accurately or efficiently.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of applyAllInputs across the codebase. rg --type ts "applyAllInputs"Length of output: 1331
| const getInputHistory = | ||
| ({ restClient }: ContractsDI) => | ||
| async (contractId: ContractId): Promise<SingleInputTx[]> => { | ||
| const transactionHeaders = await restClient.getTransactionsForContract( | ||
| contractId | ||
| ); | ||
| const transactions = await Promise.all( | ||
| transactionHeaders.transactions.map((txHeader) => | ||
| restClient.getContractTransactionById( | ||
| contractId, | ||
| txHeader.transactionId | ||
| ) | ||
| ) | ||
| ); | ||
| const sortOptionalBlock = ( | ||
| a: Option<BlockHeader>, | ||
| b: Option<BlockHeader> | ||
| ) => { | ||
| if (a._tag === "None" || b._tag === "None") { | ||
| // TODO: to avoid this error we should provide a higer level function that gets the transactions as the different | ||
| // status and with the appropiate values for each state. | ||
| throw new Error("A confirmed transaction should have a valid block"); | ||
| } else { | ||
| if (a.value.blockNo < b.value.blockNo) { | ||
| return -1; | ||
| } else if (a.value.blockNo > b.value.blockNo) { | ||
| return 1; | ||
| } else { | ||
| return 0; | ||
| } | ||
| } | ||
| }; | ||
| return transactions | ||
| .filter((tx) => tx.status === "confirmed") | ||
| .sort((a, b) => sortOptionalBlock(a.block, b.block)) | ||
| .map((tx) => { | ||
| const interval = { | ||
| from: iso8601ToPosixTime(tx.invalidBefore), | ||
| to: iso8601ToPosixTime(tx.invalidHereafter), | ||
| }; | ||
| if (tx.inputs.length === 0) { | ||
| return [{ interval }]; | ||
| } else { | ||
| return tx.inputs.map((input) => ({ input, interval })); | ||
| } | ||
| }) | ||
| .flat(); |
There was a problem hiding this comment.
The getInputHistory function retrieves and sorts transaction history for a given contract. It uses the restClient to fetch transactions and then sorts them based on block numbers. The function throws an error if a confirmed transaction does not have a valid block, which is a good practice for ensuring data integrity. However, the error message could be more descriptive, and the function could potentially handle the 'None' case more gracefully, perhaps by logging a warning and skipping the transaction rather than throwing an error, depending on the expected behavior in this scenario.
- throw new Error("A confirmed transaction should have a valid block");
+ // Consider handling the 'None' case without throwing an error, depending on the expected behavior.
+ // For example, log a warning and continue processing other transactions.Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const getInputHistory = | |
| ({ restClient }: ContractsDI) => | |
| async (contractId: ContractId): Promise<SingleInputTx[]> => { | |
| const transactionHeaders = await restClient.getTransactionsForContract( | |
| contractId | |
| ); | |
| const transactions = await Promise.all( | |
| transactionHeaders.transactions.map((txHeader) => | |
| restClient.getContractTransactionById( | |
| contractId, | |
| txHeader.transactionId | |
| ) | |
| ) | |
| ); | |
| const sortOptionalBlock = ( | |
| a: Option<BlockHeader>, | |
| b: Option<BlockHeader> | |
| ) => { | |
| if (a._tag === "None" || b._tag === "None") { | |
| // TODO: to avoid this error we should provide a higer level function that gets the transactions as the different | |
| // status and with the appropiate values for each state. | |
| throw new Error("A confirmed transaction should have a valid block"); | |
| } else { | |
| if (a.value.blockNo < b.value.blockNo) { | |
| return -1; | |
| } else if (a.value.blockNo > b.value.blockNo) { | |
| return 1; | |
| } else { | |
| return 0; | |
| } | |
| } | |
| }; | |
| return transactions | |
| .filter((tx) => tx.status === "confirmed") | |
| .sort((a, b) => sortOptionalBlock(a.block, b.block)) | |
| .map((tx) => { | |
| const interval = { | |
| from: iso8601ToPosixTime(tx.invalidBefore), | |
| to: iso8601ToPosixTime(tx.invalidHereafter), | |
| }; | |
| if (tx.inputs.length === 0) { | |
| return [{ interval }]; | |
| } else { | |
| return tx.inputs.map((input) => ({ input, interval })); | |
| } | |
| }) | |
| .flat(); | |
| const getInputHistory = | |
| ({ restClient }: ContractsDI) => | |
| async (contractId: ContractId): Promise<SingleInputTx[]> => { | |
| const transactionHeaders = await restClient.getTransactionsForContract( | |
| contractId | |
| ); | |
| const transactions = await Promise.all( | |
| transactionHeaders.transactions.map((txHeader) => | |
| restClient.getContractTransactionById( | |
| contractId, | |
| txHeader.transactionId | |
| ) | |
| ) | |
| ); | |
| const sortOptionalBlock = ( | |
| a: Option<BlockHeader>, | |
| b: Option<BlockHeader> | |
| ) => { | |
| if (a._tag === "None" || b._tag === "None") { | |
| // TODO: to avoid this error we should provide a higer level function that gets the transactions as the different | |
| // status and with the appropiate values for each state. | |
| // Consider handling the 'None' case without throwing an error, depending on the expected behavior. | |
| // For example, log a warning and continue processing other transactions. | |
| } else { | |
| if (a.value.blockNo < b.value.blockNo) { | |
| return -1; | |
| } else if (a.value.blockNo > b.value.blockNo) { | |
| return 1; | |
| } else { | |
| return 0; | |
| } | |
| } | |
| }; | |
| return transactions | |
| .filter((tx) => tx.status === "confirmed") | |
| .sort((a, b) => sortOptionalBlock(a.block, b.block)) | |
| .map((tx) => { | |
| const interval = { | |
| from: iso8601ToPosixTime(tx.invalidBefore), | |
| to: iso8601ToPosixTime(tx.invalidHereafter), | |
| }; | |
| if (tx.inputs.length === 0) { | |
| return [{ interval }]; | |
| } else { | |
| return tx.inputs.map((input) => ({ input, interval })); | |
| } | |
| }) | |
| .flat(); |
| async function createContract( | ||
| lifecycle: RuntimeLifecycle, | ||
| schema: DelayPaymentScheme, | ||
| rewardAddress?: StakeAddressBech32 | ||
| ): Promise<[ContractId, TxId]> { | ||
| const contractBundle = mkDelayPayment(schema); | ||
| const tags = mkDelayPaymentTags(schema); | ||
| // TODO: PLT-9089: Modify runtimeLifecycle.contracts.createContract to support bundle (calling createContractSources) | ||
| const contractSources = await lifecycle.restClient.createContractSources( | ||
| contractBundle | ||
| ); | ||
| const walletAddress = await lifecycle.wallet.getChangeAddress(); | ||
| const unsignedTx = await lifecycle.restClient.buildCreateContractTx({ | ||
| sourceId: contractSources.contractSourceId, | ||
| tags, | ||
| changeAddress: walletAddress, | ||
| stakeAddress: rewardAddress, | ||
| minimumLovelaceUTxODeposit: 3_000_000, | ||
| version: "v1", | ||
| }); | ||
| const signedCborHex = await lifecycle.wallet.signTx(unsignedTx.tx.cborHex); | ||
| await lifecycle.restClient.submitContract( | ||
| unsignedTx.contractId, | ||
| transactionWitnessSetTextEnvelope(signedCborHex) | ||
| ); | ||
| const txId = contractIdToTxId(unsignedTx.contractId); | ||
| return [unsignedTx.contractId, txId]; | ||
| //---------------- | ||
| } |
There was a problem hiding this comment.
The createContract function is implemented with a TODO comment indicating a pending task (PLT-9089). Ensure that this task is tracked and prioritized appropriately.
| function inputToInputContent (input: Input): InputContent { | ||
| if (input === "input_notify") { | ||
| return "input_notify"; | ||
| } | ||
| if ("that_deposits" in input) { | ||
| input | ||
| return input as IDeposit | ||
| } | ||
| if ("input_that_chooses_num" in input) { | ||
| input | ||
| return input as IChoice; | ||
| } | ||
| return "input_notify" | ||
| } |
There was a problem hiding this comment.
The inputToInputContent function has been added to convert Input to InputContent. This function appears to be incomplete as it contains lines with only the variable input (lines 726 and 730). These lines should either be removed or completed if they were intended to perform some operation.
- input
- inputCommittable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| function inputToInputContent (input: Input): InputContent { | |
| if (input === "input_notify") { | |
| return "input_notify"; | |
| } | |
| if ("that_deposits" in input) { | |
| input | |
| return input as IDeposit | |
| } | |
| if ("input_that_chooses_num" in input) { | |
| input | |
| return input as IChoice; | |
| } | |
| return "input_notify" | |
| } | |
| function inputToInputContent (input: Input): InputContent { | |
| if (input === "input_notify") { | |
| return "input_notify"; | |
| } | |
| if ("that_deposits" in input) { | |
| return input as IDeposit | |
| } | |
| if ("input_that_chooses_num" in input) { | |
| return input as IChoice; | |
| } | |
| return "input_notify" | |
| } |
| // For IDeposit and IChoice is the InputContent + the merkle object | ||
| return { | ||
| ...merkleizedHashAndContinuation, | ||
| ...content, | ||
| }; | ||
| } | ||
| } else { | ||
| return content; | ||
| } | ||
| } | ||
|
|
||
| if (isDepositAction(cse.case)) { | ||
| const deposit = cse.case; | ||
| return accumulatorFromDeposit(env, state, { | ||
| type: "Deposit", | ||
| deposit, | ||
| policyId, | ||
| applyAction() { | ||
| const input = decorateInput({ | ||
| input_from_party: deposit.party, | ||
| that_deposits: evalValue(env, state, deposit.deposits), | ||
| of_token: deposit.of_token, | ||
| into_account: deposit.into_account, | ||
| }); | ||
| // TODO: Re-check if this env should be the same as the initial env or a new one. | ||
| const appliedInput = applyAllInputs(env, state, currentContract, [ | ||
| input, | ||
| ]); | ||
|
|
||
| // TODO: Improve error handling | ||
| if (typeof appliedInput === "string") throw new Error(appliedInput); | ||
| return { | ||
| inputs: [input], | ||
| environment: env, | ||
| reducedState: appliedInput.state, | ||
| reducedContract: appliedInput.continuation, | ||
| warnings: [...previousWarnings, ...appliedInput.warnings], | ||
| payments: [...previousPayments, ...appliedInput.payments], | ||
| }; | ||
| }, | ||
| }); | ||
| } else if (isChoice(cse.case)) { | ||
| const choice = cse.case; | ||
|
|
||
| return accumulatorFromChoice({ | ||
| type: "Choice", | ||
| choice, | ||
| policyId, | ||
| applyAction(chosenNum: ChosenNum) { | ||
| if (!inBounds(chosenNum, choice.choose_between)) { | ||
| throw new Error("Chosen number is not in bounds"); | ||
| } | ||
| const input = decorateInput({ | ||
| for_choice_id: choice.for_choice, | ||
| input_that_chooses_num: chosenNum, | ||
| }); | ||
| // TODO: Re-check if this env should be the same as the initial env or a new one. | ||
| const appliedInput = applyAllInputs(env, state, currentContract, [ | ||
| input, | ||
| ]); | ||
| // TODO: Improve error handling | ||
| if (typeof appliedInput === "string") throw new Error(appliedInput); | ||
| return { | ||
| inputs: [input], | ||
| environment: env, | ||
| reducedState: appliedInput.state, | ||
| reducedContract: appliedInput.continuation, | ||
| warnings: [...previousWarnings, ...appliedInput.warnings], | ||
| payments: [...previousPayments, ...appliedInput.payments], | ||
| }; | ||
| }, | ||
| }); | ||
| } else { | ||
| const notify = cse.case; | ||
| if (!evalObservation(env, state, notify.notify_if)) { | ||
| return mergeApplicableActionAccumulator.empty; | ||
| } | ||
|
|
||
| return accumulatorFromNotify({ | ||
| type: "Notify", | ||
| policyId, | ||
| applyAction() { | ||
| const input = decorateInput("input_notify"); | ||
| // TODO: Re-check if this env should be the same as the initial env or a new one. | ||
| const appliedInput = applyAllInputs(env, state, currentContract, [ | ||
| input, | ||
| ]); | ||
| // TODO: Improve error handling | ||
| if (typeof appliedInput === "string") throw new Error(appliedInput); | ||
| return { | ||
| inputs: [input], | ||
| environment: env, | ||
| reducedState: appliedInput.state, | ||
| reducedContract: appliedInput.continuation, | ||
| warnings: [...previousWarnings, ...appliedInput.warnings], | ||
| payments: [...previousPayments, ...appliedInput.payments], | ||
| }; | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The script output now provides the complete content of the getApplicableActionFromCase function. From the code, we can observe the following:
- The function handles different case types:
Deposit,Choice, andNotify. - For each case type, there is a corresponding
applyActionfunction that processes the input and applies it to the contract state. - Error handling is present with checks like
if (typeof appliedInput === "string") throw new Error(appliedInput);andif (!inBounds(chosenNum, choice.choose_between)) { throw new Error("Chosen number is not in bounds"); }. - There are
TODOcomments indicating areas for potential improvement, particularly in error handling and environment checks.
Based on the code provided, the function appears to handle all specified case types and includes basic error handling. However, the TODO comments suggest that error handling could be further improved, and the environment handling re-checked.
Given this information, the review comment is partially verified. The function does handle all case types and includes error handling, but there are indications that error handling could be enhanced.
Analysis chain
The getApplicableActionFromCase function is another complex function that involves asynchronous calls and conditional logic based on the type of case. It's crucial to ensure that the function handles all case types correctly and that the error handling is robust, especially since it interacts with external services via restClient.
The getApplicableActionFromCase function exists in the examples/nodejs/src/experimental-features/applicable-inputs.ts file. To further analyze the handling of different case types and the robustness of error handling within the function, we need to inspect the contents of the function.
The provided script output shows the beginning of the getApplicableActionFromCase function and some of its internal logic. However, the output is truncated and does not include the entire function body, which is necessary to fully assess the handling of different case types and the error management within the function.
To complete the verification, we need to see the rest of the function body. Let's execute another script to retrieve the remaining part of the function.
The script output provides additional content from the getApplicableActionFromCase function, but it is still not the complete function. The output is cut off, and we do not have the full context to assess the handling of all case types and the error management within the function.
To ensure a thorough verification, we need to retrieve the rest of the function. Let's execute another script to get the remaining part of the function.
The script failed due to a regex parse error caused by an unclosed character class. The square brackets [ are special characters in regex and need to be escaped to be used as literal characters in the search pattern.
Let's correct the script by escaping the square brackets and attempt to retrieve the rest of the getApplicableActionFromCase function for complete analysis.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that getApplicableActionFromCase function handles all case types and has robust error handling.
ast-grep --lang typescript --pattern $'async function getApplicableActionFromCase($_, $_, $_, $_, $_, $_, $_, $_) {
$$$
}'
Length of output: 125
Script:
#!/bin/bash
# Search for the function getApplicableActionFromCase without specifying the parameter count
ast-grep --lang typescript --pattern $'async function getApplicableActionFromCase($$$) {
$$$
}'
Length of output: 98
Script:
#!/bin/bash
# Search for the function getApplicableActionFromCase within TypeScript files
rg --type ts 'async function getApplicableActionFromCase'
Length of output: 166
Script:
#!/bin/bash
# Extract the content of getApplicableActionFromCase function to analyze case handling and error management
rg --type ts --context 30 'async function getApplicableActionFromCase' examples/nodejs/src/experimental-features/applicable-inputs.ts
Length of output: 1958
Script:
#!/bin/bash
# Retrieve the remaining part of the getApplicableActionFromCase function for further analysis
rg --type ts --after-context 40 'cseContinuation = cse.then;' examples/nodejs/src/experimental-features/applicable-inputs.ts
Length of output: 1428
Script:
#!/bin/bash
# Retrieve the rest of the getApplicableActionFromCase function for complete analysis
rg --type ts --after-context 60 'applyAllInputs(env, state, currentContract, [' examples/nodejs/src/experimental-features/applicable-inputs.ts
Length of output: 295
Script:
#!/bin/bash
# Correct the regex and retrieve the rest of the getApplicableActionFromCase function for complete analysis
rg --type ts --after-context 60 'applyAllInputs\(env, state, currentContract, \[' examples/nodejs/src/experimental-features/applicable-inputs.ts
Length of output: 2773
bjornkihlberg
force-pushed
the
plt-8891-add-merkleized-contract-flow-example
branch
from
5882547 to
0a7e961
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (8)
- changelog.d/scriv.ini
- examples/nodejs/package-lock.json
- examples/nodejs/package.json
- examples/nodejs/src/tsconfig.json
- examples/nodejs/tsconfig.json
- packages/adapter/package.json
- tsconfig-base.json
- tsconfig.json
Files selected for processing (24)
- .vscode/settings.json (1 hunks)
- changelog.d/20240108_165452_hrajchert_plt_8891_add_merkleized_contract_flow_example.md (1 hunks)
- examples/nodejs/.gitignore (1 hunks)
- examples/nodejs/src/config.ts (1 hunks)
- examples/nodejs/src/experimental-features/applicable-inputs.ts (1 hunks)
- examples/nodejs/src/experimental-features/metadata.ts (1 hunks)
- examples/nodejs/src/marlowe-object-flow.ts (1 hunks)
- examples/nodejs/src/wallet-flow.ts (1 hunks)
- jsdelivr-npm-importmap.js (1 hunks)
- packages/adapter/src/bigint.ts (1 hunks)
- packages/adapter/src/time.ts (1 hunks)
- packages/language/core/v1/src/contract.ts (2 hunks)
- packages/language/core/v1/src/index.ts (1 hunks)
- packages/language/core/v1/src/inputs.ts (3 hunks)
- packages/language/core/v1/src/semantics.ts (11 hunks)
- packages/language/core/v1/src/transaction.ts (2 hunks)
- packages/marlowe-object/src/guards.ts (1 hunks)
- packages/marlowe-object/src/index.ts (1 hunks)
- packages/marlowe-object/src/object.ts (1 hunks)
- packages/runtime/client/rest/src/index.ts (3 hunks)
- packages/runtime/client/rest/src/pagination.ts (2 hunks)
- packages/runtime/lifecycle/src/api.ts (3 hunks)
- packages/runtime/lifecycle/src/generic/contracts.ts (4 hunks)
- packages/runtime/lifecycle/src/generic/runtime.ts (1 hunks)
Files skipped from review as they are similar to previous changes (23)
- changelog.d/20240108_165452_hrajchert_plt_8891_add_merkleized_contract_flow_example.md
- examples/nodejs/.gitignore
- examples/nodejs/src/config.ts
- examples/nodejs/src/experimental-features/applicable-inputs.ts
- examples/nodejs/src/experimental-features/metadata.ts
- examples/nodejs/src/marlowe-object-flow.ts
- examples/nodejs/src/wallet-flow.ts
- jsdelivr-npm-importmap.js
- packages/adapter/src/bigint.ts
- packages/adapter/src/time.ts
- packages/language/core/v1/src/contract.ts
- packages/language/core/v1/src/index.ts
- packages/language/core/v1/src/inputs.ts
- packages/language/core/v1/src/semantics.ts
- packages/language/core/v1/src/transaction.ts
- packages/marlowe-object/src/guards.ts
- packages/marlowe-object/src/index.ts
- packages/marlowe-object/src/object.ts
- packages/runtime/client/rest/src/index.ts
- packages/runtime/client/rest/src/pagination.ts
- packages/runtime/lifecycle/src/api.ts
- packages/runtime/lifecycle/src/generic/contracts.ts
- packages/runtime/lifecycle/src/generic/runtime.ts
Additional comments: 1
.vscode/settings.json (1)
- 3-17: The addition of the debug configuration for JavaScript with specific
outFilessettings looks correct. This will help in debugging the TypeScript files that are compiled to JavaScript and placed in thedistdirectories of various packages. Ensure that all the paths specified match the actual output directories of the compiled JavaScript files.
|
|
||
| ### General | ||
|
|
||
| - Feat: Added debugging configuration for VSCode. Now if you are developing with VSCode you can open the folder as a workspace and the [Javascript Debug Terminal](https://code.visualstudio.com/docs/nodejs/nodejs-debugging#_javascript-debug-terminal) will have the appropiate source maps. ([PR#136](https://github.com/input-output-hk/marlowe-ts-sdk/pull/136)). |
There was a problem hiding this comment.
Should debugging configuration really be included in the changelog?
There was a problem hiding this comment.
basically so you know this is now a thing (if not you wouldn't even have realized it)
There was a problem hiding this comment.
I wasn't able to make it work, if I can have 5 min of your time to do it @hrajchert ... once we have pushed our 2 PR for example...
hrajchert
force-pushed
the
plt-8891-add-merkleized-contract-flow-example
branch
from
0a7e961 to
0185f4e
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- examples/nodejs/src/experimental-features/applicable-inputs.ts (1 hunks)
- examples/nodejs/src/marlowe-object-flow.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- examples/nodejs/src/experimental-features/applicable-inputs.ts
Additional comments: 12
examples/nodejs/src/marlowe-object-flow.ts (12)
46-68: The
parseClifunction is well-implemented with clear help instructions and proper argument parsing.93-130: The validators for bech32 addresses, positive bigints, and future dates are correctly implemented and provide user-friendly error messages.
138-195: The
createContractMenufunction provides a clear interactive process for creating a contract, with proper validation and user feedback.203-231: The
loadContractMenufunction correctly implements the loading and validation of an existing contract, ensuring it matches the expected tags and contract source.237-311: The
contractMenufunction is well-structured, providing an interactive menu for contract interaction with clear options for checking the state and applying actions.314-348: The
mainLoopfunction provides a clear main menu loop with options to create or load contracts and exit the application, with proper error handling for closed prompts.352-377: The
DelayPaymentSchemeinterface and related types are well-defined, providing a clear structure for the delayed payment contract.379-415: The
mkDelayPaymentfunction correctly constructs a contract bundle from the given scheme, with clear labeling and structure.463-515: The
printStateandgetStatefunctions are well-implemented, providing clear logic to print and determine the state of the delay payment contract.519-561: The
mkDelayPaymentTagsandextractSchemeFromTagsfunctions are correctly implemented to handle the creation and extraction of tags for the delay payment contract.594-636: The
validateExistingContractfunction is well-implemented, ensuring that a given contract is an instance of the delay payment contract and that it shares the same sources between different runtimes.638-658: The
mainfunction is well-structured, setting up the environment and starting the main loop of the application with clear configuration and wallet setup.
| import { deepEqual } from "@marlowe.io/adapter/deep-equal"; | ||
|
|
||
| // When this script is called, start with main. | ||
| main(); |
There was a problem hiding this comment.
Given the previous comment about moving the main() function call, it seems the call is still at the top of the file. This should be addressed to prevent unexpected behavior when the module is imported elsewhere.
- main();
+ if (require.main === module) {
+ main();
+ }Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| main(); | |
| if (require.main === module) { | |
| main(); | |
| } |
| async function createContract( | ||
| lifecycle: RuntimeLifecycle, | ||
| schema: DelayPaymentScheme, | ||
| rewardAddress?: StakeAddressBech32 | ||
| ): Promise<[ContractId, TxId]> { | ||
| const contractBundle = mkDelayPayment(schema); | ||
| const tags = mkDelayPaymentTags(schema); | ||
| // TODO: PLT-9089: Modify runtimeLifecycle.contracts.createContract to support bundle (calling createContractSources) | ||
| const contractSources = | ||
| await lifecycle.restClient.createContractSources(contractBundle); | ||
| const walletAddress = await lifecycle.wallet.getChangeAddress(); | ||
| const unsignedTx = await lifecycle.restClient.buildCreateContractTx({ | ||
| sourceId: contractSources.contractSourceId, | ||
| tags, | ||
| changeAddress: walletAddress, | ||
| stakeAddress: rewardAddress, | ||
| minimumLovelaceUTxODeposit: 3_000_000, | ||
| version: "v1", | ||
| }); | ||
| const signedCborHex = await lifecycle.wallet.signTx(unsignedTx.tx.cborHex); | ||
| await lifecycle.restClient.submitContract( | ||
| unsignedTx.contractId, | ||
| transactionWitnessSetTextEnvelope(signedCborHex) | ||
| ); | ||
| const txId = contractIdToTxId(unsignedTx.contractId); | ||
| return [unsignedTx.contractId, txId]; | ||
| //---------------- | ||
| } |
There was a problem hiding this comment.
The createContract function appears to be correctly implemented for creating the contract on the blockchain. However, there is a TODO comment indicating a pending task that should be addressed.
// TODO: PLT-9089: Modify runtimeLifecycle.contracts.createContract to support bundle (calling createContractSources)Please ensure that the task referenced by PLT-9089 is tracked and prioritized appropriately.
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async function createContract( | |
| lifecycle: RuntimeLifecycle, | |
| schema: DelayPaymentScheme, | |
| rewardAddress?: StakeAddressBech32 | |
| ): Promise<[ContractId, TxId]> { | |
| const contractBundle = mkDelayPayment(schema); | |
| const tags = mkDelayPaymentTags(schema); | |
| // TODO: PLT-9089: Modify runtimeLifecycle.contracts.createContract to support bundle (calling createContractSources) | |
| const contractSources = | |
| await lifecycle.restClient.createContractSources(contractBundle); | |
| const walletAddress = await lifecycle.wallet.getChangeAddress(); | |
| const unsignedTx = await lifecycle.restClient.buildCreateContractTx({ | |
| sourceId: contractSources.contractSourceId, | |
| tags, | |
| changeAddress: walletAddress, | |
| stakeAddress: rewardAddress, | |
| minimumLovelaceUTxODeposit: 3_000_000, | |
| version: "v1", | |
| }); | |
| const signedCborHex = await lifecycle.wallet.signTx(unsignedTx.tx.cborHex); | |
| await lifecycle.restClient.submitContract( | |
| unsignedTx.contractId, | |
| transactionWitnessSetTextEnvelope(signedCborHex) | |
| ); | |
| const txId = contractIdToTxId(unsignedTx.contractId); | |
| return [unsignedTx.contractId, txId]; | |
| //---------------- | |
| } | |
| async function createContract( | |
| lifecycle: RuntimeLifecycle, | |
| schema: DelayPaymentScheme, | |
| rewardAddress?: StakeAddressBech32 | |
| ): Promise<[ContractId, TxId]> { | |
| const contractBundle = mkDelayPayment(schema); | |
| const tags = mkDelayPaymentTags(schema); | |
| // TODO: PLT-9089: Modify runtimeLifecycle.contracts.createContract to support bundle (calling createContractSources) | |
| const contractSources = | |
| await lifecycle.restClient.createContractSources(contractBundle); | |
| const walletAddress = await lifecycle.wallet.getChangeAddress(); | |
| const unsignedTx = await lifecycle.restClient.buildCreateContractTx({ | |
| sourceId: contractSources.contractSourceId, | |
| tags, | |
| changeAddress: walletAddress, | |
| stakeAddress: rewardAddress, | |
| minimumLovelaceUTxODeposit: 3_000_000, | |
| version: "v1", | |
| }); | |
| const signedCborHex = await lifecycle.wallet.signTx(unsignedTx.tx.cborHex); | |
| await lifecycle.restClient.submitContract( | |
| unsignedTx.contractId, | |
| transactionWitnessSetTextEnvelope(signedCborHex) | |
| ); | |
| const txId = contractIdToTxId(unsignedTx.contractId); | |
| return [unsignedTx.contractId, txId]; | |
| //---------------- | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- examples/nodejs/src/experimental-features/applicable-inputs.ts (1 hunks)
- packages/language/core/v1/src/index.ts (2 hunks)
- packages/language/core/v1/src/semantics.ts (13 hunks)
Files skipped from review as they are similar to previous changes (3)
- examples/nodejs/src/experimental-features/applicable-inputs.ts
- packages/language/core/v1/src/index.ts
- packages/language/core/v1/src/semantics.ts
| @@ -19,8 +19,8 @@ const configGuard = t.type({ | |||
|
|
|||
| export type Config = t.TypeOf<typeof configGuard>; | |||
|
|
|||
There was a problem hiding this comment.
the documentation in the README is the following :
{
// We use blockfrost for the lucid wallet, you can create a projectId here
// https://blockfrost.io/
"blockfrostProjectId": "YOUR_PROJECT_ID",
"blockfrostUrl": "https://cardano-preprod.blockfrost.io/api/v0",
"blockfrostNetwork": "Preprod",
// You can create this seed phrase from any wallet. Do not reuse a real wallet phrase
// for a test example.
"seedPhrase": "alpha beta delta...",
"runtimeURL": "<url to a runtime instance>"
}
and the associated guard is :
const configGuard = t.type({
blockfrostProjectId: t.string,
blockfrostUrl: t.string,
network: lucidNetworkGuard,
seedPhrase: t.string,
runtimeURL: t.string,
});
the documentation is saying blockfrostNetwork instead of network, there is a mismatch
| @@ -1,3 +1,18 @@ | |||
| { | |||
| "typescript.tsdk": "node_modules/typescript/lib" | |||
| "typescript.tsdk": "node_modules/typescript/lib", | |||
| "debug.javascript.terminalOptions": { | |||
There was a problem hiding this comment.
I have added this issue : #162 , because I'm not able to attach the debugger ti VSCode since we moved to packages, even with this PR, it doesn't work in my environment.
| ### General | ||
|
|
||
| - Feat: Added debugging configuration for VSCode. Now if you are developing with VSCode you can open the folder as a workspace and the [Javascript Debug Terminal](https://code.visualstudio.com/docs/nodejs/nodejs-debugging#_javascript-debug-terminal) will have the appropiate source maps. ([PR#136](https://github.com/input-output-hk/marlowe-ts-sdk/pull/136)). | ||
| - Feat: Started an experimental getApplicableActions that should replace the current getApplicableInputs. ([PR#136](https://github.com/input-output-hk/marlowe-ts-sdk/pull/136)) |
There was a problem hiding this comment.
Are we officially using a convention for the change log ? like Feat, Breaking Change etc ? If yes it should be documented I guess, do we have some documentation around ?
There was a problem hiding this comment.
we discussed this together with you and @bjornkihlberg.
But yes, we should/could document the structure
| * What is the new state after applying an action and reducing until quiescent | ||
| */ | ||
| reducedState: MarloweState; | ||
| /** |
There was a problem hiding this comment.
what about this instead ?
export interface AppliedActionResult {
environment: Environment;
inputs: Input[];
outputs : {
warnings: TransactionWarning[];
state: MarloweState;
contract: Contract;
payments: Payment[];
}
}There was a problem hiding this comment.
could be... I do want to start a discussion regarding this type (also why is on a separate experimental folder for the moment).
I also don't like to have the applyAction method as part of the applicableActions
|
|
||
| interface CanNotify { | ||
| type: "Notify"; | ||
| policyId: PolicyId; |
There was a problem hiding this comment.
what policyId are we talking about here ? I don't understand
There was a problem hiding this comment.
The policyId of a created contract. I need this for filtering my actions. I need to hide this away somehow
| const timeInterval = { from: now, to: nextTimeout - 1n }; | ||
| const env = environment ?? { timeInterval }; | ||
| if (typeof contractDetails.state === "undefined") { | ||
| // TODO: Check, I believe this happens when a contract is in a closed state, but it would be nice |
There was a problem hiding this comment.
When currentContract and state are undefined
} /** Only available when the contract is Unsigned/Submitted and Active */ & {
currentContract?: Contract;
state?: MarloweState;
} /** Only available when Active */ & {
utxo?: TxOutRef;
} /** Only available when the contract is confirmed */ & {
block?: BlockHeader;
} /** Only available When the contract is Unsigned/Submitted */ & {
txBody?: TextEnvelope;
};
| ? [ | ||
| { | ||
| type: "Advance", | ||
| policyId: contractDetails.roleTokenMintingPolicyId, |
There was a problem hiding this comment.
@hrajchert , I don't understand why contractDetails.roleTokenMintingPolicyId is not optional. can't we use a contract without role tokens ?
There was a problem hiding this comment.
I didn't make that a required field, it was defined required so I use it. If the contract doesn't have role tokens this part of the code should not even matter
| environment?: Environment | ||
| ): Promise<ApplicableAction[]> { | ||
| const contractDetails = await restClient.getContractById(contractId); | ||
| const currentContract = contractDetails.currentContract |
There was a problem hiding this comment.
what does it means when it is closed ? currentContract == undefined but we want it to be the initial one ? why ?
There was a problem hiding this comment.
Yeah, I need to revisit this as part of PLT-9054. But you can use this as an example of the confusing API of get contract details discussion
| const nextTimeout = getNextTimeout(currentContract, now) ?? oneDayFrom(now); | ||
| const timeInterval = { from: now, to: nextTimeout - 1n }; | ||
| const env = environment ?? { timeInterval }; | ||
| if (typeof contractDetails.state === "undefined") { |
There was a problem hiding this comment.
this check should be the first line of the function to avoid having variable above in an unpredictable state.
N.B : If we type the states of the contracts, these pieces of code could be removed
| function getTokenMap() { | ||
| if (tokenMap.size === 0 && tokens.length > 0) { | ||
| tokens.forEach((token) => { | ||
| // Role tokens only have 1 element |
There was a problem hiding this comment.
Not always I believe, Roles Tokens could have a quantity > 1
e.g : for the raffle we have duplicated the role tokens in case of the winners lost their tokens...
Should we filter out the Ada ? Same issue as there : #153
There was a problem hiding this comment.
probably, I'll do it as part of PLT-9054
| restClient: RestClient, | ||
| env: Environment, | ||
| currentContract: Contract, | ||
| state: MarloweState, |
There was a problem hiding this comment.
state -> currentState , there is an hidden concept [Contract,State] imo
There was a problem hiding this comment.
yes, I tried to name that concept in the past with no luck
| state: MarloweState, | ||
| previousPayments: Payment[], | ||
| previousWarnings: TransactionWarning[], | ||
| cse: Case, |
There was a problem hiding this comment.
I bugged on cse... could it be something more aCase ?
and cseContinuation -> aCaseContinuation
| @@ -0,0 +1,8 @@ | |||
| import { Address } from "@marlowe.io/language-core-v1"; | |||
There was a problem hiding this comment.
I don't understand the link between metadata and splitting an address in 2
| const tag = "DELAY_PYMNT-1"; | ||
| const tags = {} as Tags; | ||
|
|
||
| tags[`${tag}-from-0`] = splitAddress(schema.payFrom)[0]; |
There was a problem hiding this comment.
if your intent is to store only metadata more than tagging you can simply create a tag like that :
{ "myDappId" :{ schema : schema}}There was a problem hiding this comment.
This can (and will) be improved of course. There is one discussion I want to have about TemplateParameters/Scheme to add a DSL to define it and to be able to share it inside the metadata of contract creation.
I'm abusing tags here really, Ideally I should have a single tag DELAY_PAYMENT: v1, and then have all the scheme encoded efficiently inside the metadata.
The tag should only help me easily find the type of contracts that I want.
Encoding the scheme in the metadata is a simple way to share the contract sources between unconnected runtime instances. The approach you suggest doesn't cope with the max size of the value. Note that I even have to split the address in half to cope with this limitation.
| @@ -17,7 +17,7 @@ export type ItemRange = t.TypeOf<typeof ItemRangeGuard>; | |||
| export const itemRanged = (s: string) => unsafeEither(ItemRangeGuard.decode(s)); | |||
|
|
|||
| export interface Page { | |||
| current: ItemRange; | |||
| current?: ItemRange; | |||
There was a problem hiding this comment.
in which case we don't have current ? when there is only 1 page ? the semantic is a bit strange, we could consider than there is always a currentPage and handle that case....
There was a problem hiding this comment.
when there arent any transactions for example.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files selected for processing (3)
- examples/nodejs/Readme.md (1 hunks)
- examples/nodejs/src/experimental-features/applicable-inputs.ts (1 hunks)
- examples/nodejs/src/marlowe-object-flow.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- examples/nodejs/Readme.md
Additional comments: 3
examples/nodejs/src/experimental-features/applicable-inputs.ts (3)
193-220: The
mkPartyFilterfunction has been updated with a caching mechanism for tokens. Ensure that this optimization does not introduce any unintended side effects and that it is tested thoroughly.120-190: The
getApplicableActionsfunction has been updated. Review the logic to ensure that it correctly handles the various states of a contract and returns the appropriate actions. Pay special attention to error handling and the use ofreduceContractUntilQuiescent.384-511: The
getApplicableActionFromCasefunction has been updated. Ensure that the logic for handling different case types (Deposit,Choice,Notify) is correct and that the error handling is robust.
| // When this script is called, start with main. | ||
| main(); |
There was a problem hiding this comment.
The previous comment about moving the main() function call to prevent unexpected behavior when the module is imported elsewhere has not been addressed. This should be fixed to ensure that main() is only called when the script is run directly.
- main();
+ if (require.main === module) {
+ main();
+ }Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| // When this script is called, start with main. | |
| main(); | |
| // When this script is called, start with main. | |
| if (require.main === module) { | |
| main(); | |
| } |
| async function createContract( | ||
| lifecycle: RuntimeLifecycle, | ||
| schema: DelayPaymentScheme, | ||
| rewardAddress?: StakeAddressBech32 | ||
| ): Promise<[ContractId, TxId]> { | ||
| const contractBundle = mkDelayPayment(schema); | ||
| const tags = mkDelayPaymentTags(schema); | ||
| // TODO: PLT-9089: Modify runtimeLifecycle.contracts.createContract to support bundle (calling createContractSources) |
There was a problem hiding this comment.
The createContract function has a TODO comment about modifying runtimeLifecycle.contracts.createContract to support a bundle. Ensure that the task referenced by PLT-9089 is tracked and prioritized appropriately.
| lifecycle: RuntimeLifecycle, | ||
| schema: DelayPaymentScheme, | ||
| rewardAddress?: StakeAddressBech32 | ||
| ): Promise<[ContractId, TxId]> { | ||
| const contractBundle = mkDelayPayment(schema); | ||
| const tags = mkDelayPaymentTags(schema); | ||
| // TODO: PLT-9089: Modify runtimeLifecycle.contracts.createContract to support bundle (calling createContractSources) | ||
| const contractSources = | ||
| await lifecycle.restClient.createContractSources(contractBundle); | ||
| const walletAddress = await lifecycle.wallet.getChangeAddress(); | ||
| const unsignedTx = await lifecycle.restClient.buildCreateContractTx({ | ||
| sourceId: contractSources.contractSourceId, | ||
| tags, | ||
| changeAddress: walletAddress, | ||
| stakeAddress: rewardAddress, | ||
| minimumLovelaceUTxODeposit: 3_000_000, | ||
| version: "v1", | ||
| }); | ||
| const signedCborHex = await lifecycle.wallet.signTx(unsignedTx.tx.cborHex); | ||
| await lifecycle.restClient.submitContract( | ||
| unsignedTx.contractId, | ||
| transactionWitnessSetTextEnvelope(signedCborHex) | ||
| ); | ||
| const txId = contractIdToTxId(unsignedTx.contractId); | ||
| return [unsignedTx.contractId, txId]; | ||
| //---------------- | ||
| } | ||
|
|
||
| type ValidationResults = "InvalidTags" | "InvalidContract" | DelayPaymentScheme; | ||
|
|
||
| /** | ||
| * This function checks if the contract with the given id is an instance of the delay payment contract | ||
| * @param lifecycle | ||
| * @param contractId | ||
| * @returns | ||
| */ | ||
| async function validateExistingContract( | ||
| lifecycle: RuntimeLifecycle, | ||
| contractId: ContractId | ||
| ): Promise<ValidationResults> { | ||
| // First we try to fetch the contract details and the required tags | ||
| const contractDetails = | ||
| await lifecycle.restClient.getContractById(contractId); | ||
|
|
||
| const scheme = extractSchemeFromTags(contractDetails.tags); | ||
|
|
||
| if (!scheme) { | ||
| return "InvalidTags"; | ||
| } | ||
|
|
||
| // If the contract seems to be an instance of the contract we want (meanin, we were able | ||
| // to retrieve the contract scheme) we check that the actual initial contract has the same | ||
| // sources. | ||
| // This has 2 purposes: | ||
| // 1. Make sure we are interacting with the expected contract | ||
| // 2. Share the same sources between different Runtimes. | ||
| // When a contract source is uploaded to the runtime, it merkleizes the source code, | ||
| // but it doesn't share those sources with other runtime instances. One option would be | ||
| // to download the sources from the initial runtime and share those with another runtime. | ||
| // Or this option which doesn't require runtime to runtime communication, and just requires | ||
| // the dapp to be able to recreate the same sources. | ||
| const contractBundle = mkDelayPayment(scheme); | ||
| const { contractSourceId } = | ||
| await lifecycle.restClient.createContractSources(contractBundle); | ||
| const initialContract = await lifecycle.restClient.getContractSourceById({ | ||
| contractSourceId, | ||
| }); | ||
|
|
||
| if (!deepEqual(initialContract, contractDetails.initialContract)) { | ||
| return "InvalidContract"; | ||
| } | ||
| return scheme; | ||
| } | ||
|
|
||
| async function main() { | ||
| const args = parseCli(); | ||
| const config = await readConfig(args["--config"]); | ||
| const lucid = await Lucid.new( | ||
| new Blockfrost(config.blockfrostUrl, config.blockfrostProjectId), | ||
| config.network | ||
| ); | ||
| lucid.selectWalletFromSeed(config.seedPhrase); | ||
| const rewardAddressStr = await lucid.wallet.rewardAddress(); | ||
| const rewardAddress = rewardAddressStr | ||
| ? stakeAddressBech32(rewardAddressStr) | ||
| : undefined; | ||
| const runtimeURL = config.runtimeURL; | ||
|
|
||
| const wallet = mkLucidWallet(lucid); | ||
|
|
||
| const lifecycle = mkRuntimeLifecycle({ | ||
| runtimeURL, | ||
| wallet, | ||
| }); | ||
| try { | ||
| await mainLoop(lifecycle, rewardAddress); | ||
| } catch (e) { | ||
| console.log(`Error : ${JSON.stringify(e, null, 4)}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
Overall, the script is well-structured and follows the interactive CLI pattern using inquirer.js. The utility functions are modular and maintainable. Ensure that the TODO comment in the createContract function is addressed and that the main() function is only called when the script is run directly.
| TODO: After PR-8891 and as part of PLT-9054 move these to the runtime-lifecycle package | ||
| and create a github discussion to modify the backend. |
There was a problem hiding this comment.
Please ensure that the TODO task mentioned here is tracked in the project's issue tracking system to avoid it being overlooked.
| function isDepositAction(action: Action): action is Deposit { | ||
| return "party" in action; |
There was a problem hiding this comment.
The isDepositAction function may incorrectly identify an action as a Deposit based solely on the presence of a party property. Consider refining the type guard to accurately distinguish Deposit actions.
This PR creates a node.js interactive app that allows you to create and interact with a marlowe-object contract.
A full list of changes can be found in the scriv entry.
To test this PR you can:
resolves #125
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
await.mainfunction of wallet example.Documentation & Style
Refactor
Chores
.gitignoreto better exclude config files.