First version of the new applicable actions API #187
Conversation
WalkthroughThe updates across various packages focus on enhancing the Marlowe project's contract interaction, documentation clarity, and lifecycle management. Key improvements include refining action applicability logic, updating documentation links, refining API interfaces for contract interactions, and introducing new functionalities for handling payouts and applicable actions. These changes aim to streamline user experience and developer integration, ensuring a more intuitive and efficient interaction with Marlowe contracts. Changes
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: 10
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
packages/runtime/lifecycle/typedoc.jsonis excluded by:!**/*.json
Files selected for processing (11)
- examples/nodejs/src/marlowe-object-flow.ts (2 hunks)
- packages/marlowe-object/Readme.md (1 hunks)
- packages/runtime/client/rest/src/contract/transaction/endpoints/collection.ts (2 hunks)
- packages/runtime/client/rest/src/index.ts (1 hunks)
- packages/runtime/lifecycle/Readme.md (1 hunks)
- packages/runtime/lifecycle/src/api.ts (9 hunks)
- packages/runtime/lifecycle/src/browser/index.ts (5 hunks)
- packages/runtime/lifecycle/src/generic/applicable-actions.ts (1 hunks)
- packages/runtime/lifecycle/src/generic/runtime.ts (2 hunks)
- packages/runtime/lifecycle/src/index.ts (3 hunks)
- packages/testing-kit/src/wallet/lucid/index.ts (1 hunks)
Additional comments: 67
packages/runtime/lifecycle/Readme.md (1)
- 6-6: The update to the README.md file provides a clear and concise description of how to construct the
RuntimeLifecycleAPI, which is beneficial for users. This change enhances the documentation by focusing on the main exported type and its construction methods.packages/runtime/lifecycle/src/generic/runtime.ts (2)
- 8-8: The addition of the
mkApplicableActionsAPIimport correctly expands the functionality ofRuntimeLifecycleto include applicable actions, aligning with the PR's objectives.- 20-20: Ensure the integration of
applicableActionsin themkRuntimeLifecyclefunction is thoroughly tested to confirm it works as expected with the rest of theRuntimeLifecyclefunctionalities.packages/runtime/lifecycle/src/index.ts (3)
- 35-35: The addition of the
@category RuntimeLifecycletag to theRuntimeLifecycleOptionsinterface improves documentation by categorizing related entities, making it easier for users to navigate.- 51-51: The addition of the
@category RuntimeLifecycletag to themkRuntimeLifecyclefunction enhances the documentation by categorizing it appropriately.- 60-65: The update to the
mkRuntimeLifecyclefunction signature, including a default value for thestrictparameter, improves usability by providing sensible defaults. Ensure this change is thoroughly tested to confirm it behaves as expected.packages/marlowe-object/Readme.md (1)
- 3-5: The updates to the links and the additional details provided on bundle types enhance the documentation by ensuring accuracy and comprehensiveness, which can help users better understand and utilize the package.
packages/testing-kit/src/wallet/lucid/index.ts (1)
- 4-5: The updates to the documentation links, including adding
!at the end, correct the links and potentially fix issues with link navigation. Ensure these links lead to the intended destinations.packages/runtime/lifecycle/src/browser/index.ts (3)
- 1-37: The addition of documentation comments and a helper function to create a
RuntimeLifecycleusing a CIP-30 wallet in the browser significantly improves the documentation's clarity and expands the package's functionality.- 59-59: The introduction of the
BrowserRuntimeLifecycleOptionsinterface with a new category tag enhances the documentation by categorizing related entities, making it easier for users to navigate.- 111-116: The adjustment to the
mkRuntimeLifecyclefunction signature, including a default value for thestrictparameter, improves usability by providing sensible defaults. Ensure this change is thoroughly tested to confirm it behaves as expected.packages/runtime/client/rest/src/contract/transaction/endpoints/collection.ts (1)
- 127-141: The renaming of
TransactionTextEnvelopetoApplyInputsToContractResponseand the update to its definition to includeTextEnvelopecorrectly align with the PR's objectives of enhancing the handling of applicable actions. Ensure the integration and testing of this updated interface are thoroughly conducted.packages/runtime/lifecycle/src/api.ts (5)
- 25-39: The import statements for
ApplicableActionsAPIand related types are correctly structured and follow best practices for modularity and clarity. However, ensure that thegeneric/applicable-actions.jsmodule exports all these types as expected to avoid any import errors.- 64-79: The
RuntimeLifecycleinterface has been expanded to includepayoutsandapplicableActionsproperties. This change enhances the interface's functionality and aligns with the PR's objectives to improve contract interaction and applicable actions handling. Ensure that all instances whereRuntimeLifecycleis used are updated to accommodate these new properties.Verification successful
To further verify the accommodation of the new
payoutsandapplicableActionsproperties in theRuntimeLifecycleinterface, we need to inspect howRuntimeLifecycleis instantiated, particularly in the implementations ofmkRuntimeLifecycle. Let's examine these implementations and any related usage patterns to ensure they are updated to include the new properties.
The review comment is verified as accurate. The
RuntimeLifecycleinterface's expansion to includepayoutsandapplicableActionsproperties is properly accommodated in the instantiation ofRuntimeLifecycle, particularly in the generic implementation. This aligns with the PR's objectives to enhance contract interaction and applicable actions handling.* 90-104: > 📝 **NOTE** > This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [93-294]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instances of RuntimeLifecycle being instantiated or used to ensure they accommodate the new properties. rg --type ts "RuntimeLifecycle"Length of output: 7080
Script:
#!/bin/bash # Inspect the implementations of mkRuntimeLifecycle and related usage patterns rg --type ts "mkRuntimeLifecycle" -A 10Length of output: 15519
The new types and interfaces for contract creation (
CreateContractRequest,CreateContractRequestFromContract,CreateContractRequestFromBundle,CreateContractRequestBase) are well-structured and provide clear, detailed documentation comments. These additions significantly improve the API's expressiveness and usability for contract creation. However, ensure that the documentation links (e.g.,@link @marlowe.io/runtime-rest-client!contract.useMintedRoles) are valid and accessible.
- 291-310: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [295-360]
The
ContractsAPIinterface has been expanded with new methods (createContract,applyInputs,getApplicableInputs,getContractIds,getInputHistory) that enhance the API's capabilities for contract interaction. The documentation comments are clear and informative. However, the@deprecatedtag ongetApplicableInputssuggests that users should be aware of its future removal and consider alternative methods for obtaining applicability and reducibility information.
- 408-408: The
Filterstype provides a structured way to filter payouts, enhancing the API's usability. The design is consistent with the rest of the API changes and supports the PR's objectives to improve payouts handling. Ensure that the implementation ofbyMyRoleTokensfunction correctly filters payouts based on role tokens owned in the connected wallet.examples/nodejs/src/marlowe-object-flow.ts (3)
- 28-32: The import statements for
RuntimeLifecycle,ApplicableAction,CanDeposit, andCanAdvancefrom@marlowe.io/runtime-lifecycle/apiare correctly structured and follow best practices for modular imports. However, ensure that these newly introduced or refactored types are consistently used throughout the codebase to avoid type mismatches or runtime errors.- 268-275: The logic for fetching applicable actions and filtering them based on the contract details is clear and concise. However, it's important to ensure that the
mkFilterfunction is thoroughly tested, especially since it dynamically generates a filter function based on contract details. This is crucial for maintaining the correctness and reliability of the action filtering process.- 316-338: The switch statement handling the selected action from the Inquirer.js prompt is well-structured and covers all expected action types. However, ensure that the
applyInputsmethod in thelifecycle.contractsobject properly handles all possible errors, especially network-related issues, to prevent the script from crashing unexpectedly. Adding try-catch blocks around asynchronous calls and providing meaningful error messages can enhance the user experience and maintainability of the script.Verification successful
The
applyInputsmethod in thelifecycle.contractsobject does incorporate error handling mechanisms, leveragingfp-ts'stryCatchDefaultandpipeto handle errors, including potentially network-related ones. This aligns with the recommendation to ensure proper error handling to prevent the script from crashing unexpectedly. Therefore, the original review comment has been addressed in the implementation.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for error handling in the applyInputs method implementation. rg --type ts "applyInputs" src/@marlowe.io/runtime-lifecycle/Length of output: 136
Script:
#!/bin/bash # Search for the applyInputs method implementation across the entire repository. rg --type ts "applyInputs" --files-with-matchesLength of output: 454
Script:
#!/bin/bash # Examine the applyInputs method implementation for error handling in the identified source files. cat packages/runtime/lifecycle/src/api.ts cat packages/runtime/lifecycle/src/generic/contracts.tsLength of output: 25479
packages/runtime/client/rest/src/index.ts (45)
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The file header provides a clear introduction to the purpose of the module and how to import it, which is good for maintainability and usability.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [11-11]
The import of
axiosis correctly used for making HTTP requests. This is a common and reliable choice for interacting with REST APIs.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [12-12]
The imports from
fp-tsandio-tsare used for functional programming patterns and runtime type checking, respectively. These libraries are well-suited for building robust and type-safe applications.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [14-14]
The import of
MarloweJSONCodecfrom@marlowe.io/adapter/codecsuggests that the module is responsible for encoding and decoding JSON data, which is crucial for the correct serialization and deserialization of data exchanged with the Marlowe Runtime REST API.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [16-25]
The imports from various endpoint collections (
Payouts,Withdrawal,Contract, etc.) are organized and clearly indicate the modular structure of the API endpoints. This organization enhances the readability and maintainability of the code.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [28-28]
The import of
ContractDetailsandTransactionDetailsindicates that the module deals with detailed information about contracts and transactions, which is essential for providing a comprehensive API client.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [29-29]
The import of
RuntimeStatusandhealthcheckfrom./runtime/status.jssuggests that the module includes functionality for checking the health and status of the Marlowe Runtime, which is a good practice for ensuring the availability and reliability of the service.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [30-32]
The imports related to runtime version handling (
CompatibleRuntimeVersionGuard,RuntimeVersion) are crucial for ensuring compatibility between the client and the Marlowe Runtime. This demonstrates attention to forward and backward compatibility issues.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [33-33]
The import of
InvalidTypeErrorandstrictDynamicTypeCheckfrom@marlowe.io/adapter/io-tsis used for runtime type checking and error handling. This is an important aspect of building a robust and type-safe application, especially when dealing with external APIs.
- 38-43: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [35-43]
The exports related to pagination (
Page,ItemRange, etc.) are clearly defined and exported, which is essential for handling large datasets and providing a user-friendly API.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [45-50]
The exports related to runtime (
RuntimeStatus,RuntimeVersion, etc.) are crucial for clients to interact with the Marlowe Runtime effectively. These exports provide necessary types and utilities for managing the runtime environment.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [52-52]
The
RestClientinterface is well-documented, providing clear descriptions for each method and its purpose. This is essential for developers to understand how to use the API client effectively.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [54-54]
The method
healthcheckin theRestClientinterface is correctly documented, indicating its purpose to check the health of the Marlowe API. This is a fundamental feature for any API client.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [56-56]
The method
versionin theRestClientinterface is essential for retrieving the version of the connected runtime, which is important for compatibility checks.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [58-58]
The method
getContractsin theRestClientinterface is well-documented, including information about optional filtering and pagination options. This demonstrates attention to usability and flexibility in retrieving contracts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [60-60]
The method
buildCreateContractTxin theRestClientinterface is crucial for building transactions to create Marlowe contracts. The documentation provides clear information about the request parameters and the expected response, which is essential for developers to understand how to use this functionality.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [62-62]
The method
createContractSourcesin theRestClientinterface deals with uploading a marlowe-object bundle to the runtime. This functionality is important for managing contract sources and demonstrates the API client's capability to handle complex contract interactions.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [64-64]
The method
getContractSourceByIdin theRestClientinterface is essential for retrieving contracts based on their source ID. This functionality is crucial for developers to access specific contracts and their details.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [66-66]
The method
getContractSourceAdjacencyin theRestClientinterface provides functionality to get adjacent contract source IDs, which is important for understanding the relationships between contracts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [68-68]
The method
getContractSourceClosurein theRestClientinterface allows retrieving the full hierarchy of a contract source. This is a valuable feature for developers to understand the structure and dependencies of contracts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [70-70]
The method
getNextStepsForContractin theRestClientinterface is crucial for determining the next possible actions for a contract. This functionality is essential for building interactive and dynamic applications that interact with Marlowe contracts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [72-72]
The method
getContractByIdin theRestClientinterface provides a straightforward way to retrieve a single contract by its ID. This is a fundamental feature for accessing specific contract details.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [74-74]
The method
submitContractin theRestClientinterface is essential for submitting signed contract creation transactions. This functionality is crucial for deploying contracts to the blockchain.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [76-76]
The method
getTransactionsForContractin theRestClientinterface provides functionality to retrieve a list of transactions for a given contract. This is important for tracking the history and state changes of contracts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [78-78]
The method
applyInputsToContractin theRestClientinterface is crucial for creating transactions that apply inputs to a contract. This functionality is essential for interacting with and advancing the state of Marlowe contracts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [80-80]
The method
submitContractTransactionin theRestClientinterface is important for submitting signed transactions that apply inputs to a contract. This functionality enables the execution of contract actions on the blockchain.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [82-82]
The method
getContractTransactionByIdin theRestClientinterface provides the ability to retrieve full transaction details for a specific applyInput transaction of a contract. This is valuable for understanding the effects of specific actions on a contract.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [84-84]
The method
withdrawPayoutsin theRestClientinterface is crucial for building transactions to withdraw available payouts from a contract. This functionality is essential for managing payouts in role-based contracts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [86-86]
The method
getWithdrawalsin theRestClientinterface provides functionality to get published withdrawal transactions. This is important for tracking withdrawals and ensuring that payouts are processed correctly.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [88-88]
The method
getWithdrawalByIdin theRestClientinterface is essential for retrieving specific withdrawal transactions by ID. This functionality is crucial for managing and verifying withdrawals.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [90-90]
The method
submitWithdrawalin theRestClientinterface is important for submitting signed transactions that withdraw payouts from a contract. This functionality enables the actual withdrawal of funds from contracts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [92-92]
The method
getPayoutsin theRestClientinterface provides functionality to retrieve payouts to parties from role-based contracts. This is crucial for managing and distributing payouts according to contract terms.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [94-94]
The method
getPayoutByIdin theRestClientinterface is essential for retrieving payout information associated with a specific payout ID. This functionality is important for tracking and managing individual payouts.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [96-96]
The
mkRestClientArgumentDynamicTypeCheckfunction is used to perform runtime type checking on thebaseURLargument. This is a good practice for ensuring that the provided argument meets the expected type requirements.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [98-98]
The
withDynamicTypeCheckfunction is a higher-order function that wraps another function with runtime type checking. This is a powerful pattern for ensuring type safety and providing helpful error messages when type mismatches occur.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [100-100]
The
mkRestClientfunction is overloaded to support optionalstricttype checking. This flexibility allows developers to choose between strict runtime type checking and potentially improved performance without type checking.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [102-102]
The implementation of
mkRestClientincludes dynamic type checking for bothbaseURLandstrictarguments, ensuring that they meet the expected types. This is crucial for preventing runtime errors and ensuring the correct operation of the REST client.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [104-104]
The creation of an
axiosInstancewithinmkRestClientis correctly configured withbaseURLand custom transformers for request and response data usingMarloweJSONCodec. This setup is essential for ensuring that data exchanged with the Marlowe Runtime REST API is correctly encoded and decoded.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [106-106]
The caching of the runtime version within
mkRestClientis a smart optimization, as the runtime version is not expected to change during the lifetime of the REST client. This reduces unnecessary API calls and improves performance.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [108-108]
The check for compatibility with the runtime version using
CompatibleRuntimeVersionGuardis an important safety measure. It ensures that the REST client is compatible with the version of the Marlowe Runtime it is connecting to, preventing potential issues due to version mismatches.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [110-110]
The implementation of each method in the
RestClientinterface withinmkRestClientis well-structured and makes use of thewithDynamicTypeCheckfunction for runtime type checking. This ensures that the methods are type-safe and provide helpful error messages in case of type mismatches.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [114-114]
The
PayoutsAPI,WithdrawalsAPI, andContractsAPIinterfaces are marked as hidden, suggesting that they are intended for internal use within the module. It's important to ensure that these interfaces are well-documented and maintained, even if they are not exposed to external consumers.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [116-116]
The
RestDItype is used for dependency injection, providing a way to inject different implementations of the REST client API. This is a good practice for enabling flexibility and testability in the application.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [118-118]
The
FPTSRestAPIinterface provides a legacy version of the REST client API usingfp-tstypes. While maintaining legacy interfaces is important for backward compatibility, it's crucial to ensure that they are kept up to date and deprecated in a controlled manner.
- 38-43: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [120-120]
The
mkFPTSRestClientfunction provides an implementation of theFPTSRestAPIinterface, usingfp-tstypes for functional programming patterns. This demonstrates attention to providing alternative implementations for different programming paradigms.packages/runtime/lifecycle/src/generic/applicable-actions.ts (2)
- 7-27: The imports from
@marlowe.io/language-core-v1are extensive and cover a wide range of functionalities. Ensure that all imported entities are used within this module to avoid unnecessary dependencies that could impact bundle size and load times.Verification successful
To verify the usage of imported entities within
packages/runtime/lifecycle/src/generic/applicable-actions.ts, I will search for references to each imported entity within the file. This will help determine if all imported entities are indeed utilized.
The verification process has confirmed that all imported entities from
@marlowe.io/language-core-v1inpackages/runtime/lifecycle/src/generic/applicable-actions.tsare utilized within the file. This addresses the concern about unnecessary dependencies impacting bundle size and load times.* 813-824: The documentation comments for the `ContractDetails` type and its related types (`ClosedContract` and `ActiveContract`) provide valuable context for understanding the API's design. However, ensure that these comments are kept up-to-date as the API evolves to avoid confusion among users and maintainers.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all imported entities are utilized within the file. ast-grep --lang typescript --pattern $'import { $$$ } from "@marlowe.io/language-core-v1"'Length of output: 9386
Script:
#!/bin/bash # Search for references to each imported entity within applicable-actions.ts entities=("MarloweState" "Party" "Contract" "Deposit" "Choice" "Input" "ChosenNum" "Environment" "Timeout" "getNextTimeout" "datetoTimeout" "Case" "Action" "Notify" "InputContent" "RoleName" "Token" "Bound" "BuiltinByteString") for entity in "${entities[@]}"; do echo "Searching for $entity in applicable-actions.ts" rg --count "$entity" ./packages/runtime/lifecycle/src/generic/applicable-actions.ts doneLength of output: 4538
The documentation comments here are clear and provide useful context. Just ensure they are kept up-to-date with any future changes.
| value: | ||
| | CanDeposit | ||
| | CanAdvance | ||
| | { actionType: "check-state" } | ||
| | { actionType: "return" }; | ||
| }> = [ | ||
| { | ||
| name: "Re-check contract state", | ||
| value: { actionType: "check-state", results: undefined }, | ||
| value: { actionType: "check-state" }, | ||
| }, | ||
| ...myActions.map((action) => { | ||
| switch (action.type) { | ||
| switch (action.actionType) { | ||
| case "Advance": | ||
| return { | ||
| name: "Close contract", | ||
| description: | ||
| contractState.type == "PaymentMissed" | ||
| ? "The payer will receive minUTXO" | ||
| : "The payer will receive minUTXO and the payee will receive the payment", | ||
| value: { actionType: "advance", results: action.applyAction() }, | ||
| value: action, | ||
| }; | ||
|
|
||
| case "Deposit": | ||
| return { | ||
| name: `Deposit ${action.deposit.deposits} lovelaces`, | ||
| value: { actionType: "deposit", results: action.applyAction() }, | ||
| value: action, | ||
| }; | ||
| default: | ||
| throw new Error("Unexpected action type"); | ||
| } | ||
| }), | ||
| { | ||
| name: "Return to main menu", | ||
| value: { actionType: "return", results: undefined }, | ||
| value: { actionType: "return" }, |
There was a problem hiding this comment.
The construction of the choices array for the Inquirer.js prompt is well-implemented, dynamically generating choices based on the applicable actions. However, the default case in the switch statement throws an error for unexpected action types. Consider adding a more descriptive error message that includes the unexpected action type to aid in debugging.
- throw new Error("Unexpected action type");
+ throw new Error(`Unexpected action type: ${action.actionType}`);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.
| value: | |
| | CanDeposit | |
| | CanAdvance | |
| | { actionType: "check-state" } | |
| | { actionType: "return" }; | |
| }> = [ | |
| { | |
| name: "Re-check contract state", | |
| value: { actionType: "check-state", results: undefined }, | |
| value: { actionType: "check-state" }, | |
| }, | |
| ...myActions.map((action) => { | |
| switch (action.type) { | |
| switch (action.actionType) { | |
| case "Advance": | |
| return { | |
| name: "Close contract", | |
| description: | |
| contractState.type == "PaymentMissed" | |
| ? "The payer will receive minUTXO" | |
| : "The payer will receive minUTXO and the payee will receive the payment", | |
| value: { actionType: "advance", results: action.applyAction() }, | |
| value: action, | |
| }; | |
| case "Deposit": | |
| return { | |
| name: `Deposit ${action.deposit.deposits} lovelaces`, | |
| value: { actionType: "deposit", results: action.applyAction() }, | |
| value: action, | |
| }; | |
| default: | |
| throw new Error("Unexpected action type"); | |
| } | |
| }), | |
| { | |
| name: "Return to main menu", | |
| value: { actionType: "return", results: undefined }, | |
| value: { actionType: "return" }, | |
| value: | |
| | CanDeposit | |
| | CanAdvance | |
| | { actionType: "check-state" } | |
| | { actionType: "return" }; | |
| }> = [ | |
| { | |
| name: "Re-check contract state", | |
| value: { actionType: "check-state" }, | |
| }, | |
| ...myActions.map((action) => { | |
| switch (action.actionType) { | |
| case "Advance": | |
| return { | |
| name: "Close contract", | |
| description: | |
| contractState.type == "PaymentMissed" | |
| ? "The payer will receive minUTXO" | |
| : "The payer will receive minUTXO and the payee will receive the payment", | |
| value: action, | |
| }; | |
| case "Deposit": | |
| return { | |
| name: `Deposit ${action.deposit.deposits} lovelaces`, | |
| value: action, | |
| }; | |
| default: | |
| throw new Error(`Unexpected action type: ${action.actionType}`); | |
| } | |
| }), | |
| { | |
| name: "Return to main menu", | |
| value: { actionType: "return" }, |
| /** | ||
| This is an experimental module that aims to replace the current `next` features. | ||
| 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.
The module header comments mention a TODO related to moving these functionalities to the runtime-lifecycle package after PR-8891 and as part of PLT-9054. It's important to track this task in a project management tool or a GitHub issue to ensure it doesn't get overlooked.
Would you like me to create a GitHub issue to track this task?
| export interface ApplicableActionsAPI { | ||
| // TODO: Maybe we want to add an applyAction or applyInput here instead or in addition to getInput. |
There was a problem hiding this comment.
The TODO comment within the ApplicableActionsAPI interface suggests considering the addition of an applyAction or applyInput method. It's crucial to evaluate the API's design comprehensively to ensure it meets the users' needs and to document any decisions or future plans regarding this suggestion.
Would you like to discuss the potential addition of these methods further?
| */ | ||
| getInputHistory(contractId: ContractId): Promise<SingleInputTx[]>; | ||
| } | ||
|
|
||
| /** | ||
| * @hidden | ||
| */ | ||
| export type PayoutsDI = WalletDI & RestDI; | ||
|
|
||
| /** | ||
| * @category PayoutsAPI | ||
| */ | ||
| export interface PayoutsAPI { | ||
| /** | ||
| * Provide All the availaible payouts for the connected Wallet |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [367-395]
The PayoutsAPI interface and the onlyByContractIds function introduce new functionalities for handling payouts, including retrieving available and withdrawn payouts and filtering them by contract IDs or role tokens. These additions are in line with the PR's objectives to enhance payouts handling. Ensure that the TODO comment in the withdraw method is addressed or tracked in a separate issue for future resolution.
Would you like me to open a GitHub issue to track the resolution of the TODO comment in the withdraw method?
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (13)
- changelog.d/20240304_130046_hrajchert_applicable_inputs_refactor.md (1 hunks)
- examples/nodejs/src/marlowe-object-flow.ts (2 hunks)
- packages/language/core/v1/Readme.md (1 hunks)
- packages/language/core/v1/src/next/applicables/canChoose.ts (1 hunks)
- packages/language/core/v1/src/next/applicables/canDeposit.ts (2 hunks)
- packages/language/core/v1/src/next/applicables/canNotify.ts (1 hunks)
- packages/language/core/v1/src/next/applicables/index.ts (1 hunks)
- packages/language/core/v1/src/next/common/IsMerkleizedContinuation.ts (1 hunks)
- packages/language/core/v1/src/next/common/caseIndex.ts (1 hunks)
- packages/language/core/v1/src/next/index.ts (3 hunks)
- packages/runtime/lifecycle/src/api.ts (9 hunks)
- packages/runtime/lifecycle/src/generic/applicable-actions.ts (1 hunks)
- packages/runtime/lifecycle/src/generic/runtime.ts (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- examples/nodejs/src/marlowe-object-flow.ts
- packages/runtime/lifecycle/src/api.ts
- packages/runtime/lifecycle/src/generic/runtime.ts
Additional comments: 8
changelog.d/20240304_130046_hrajchert_applicable_inputs_refactor.md (1)
- 1-4: The changelog entries accurately reflect the additions and improvements made in this PR. It's clear and concise, providing a good overview of the changes.
packages/language/core/v1/src/next/common/caseIndex.ts (1)
- 3-9: The deprecation of
CaseIndexis clearly marked, and guidance is provided to use the new@marlowe.io/runtime-lifecycle!api.ApplicableActionsAPI. This is a good practice for transitioning to new APIs.packages/language/core/v1/src/next/common/IsMerkleizedContinuation.ts (1)
- 3-9: The deprecation of
IsMerkleizedContinuationis handled consistently with other deprecations, providing clear guidance towards the new API. This consistency is beneficial for maintainability.packages/language/core/v1/src/next/applicables/index.ts (1)
- 7-13: The deprecation of
ApplicableInputsis clearly communicated, guiding developers towards the new API. This approach ensures backward compatibility while encouraging the use of the new API.packages/language/core/v1/src/next/applicables/canNotify.ts (1)
- 6-19: The deprecation of
CanNotifyand related functions is clearly communicated. However, ensure that the simplification of thetoInputfunction to return a static string does not negatively impact existing functionality.packages/language/core/v1/src/next/applicables/canChoose.ts (1)
- 7-23: The deprecation of
CanChooseand related functions is handled consistently with other deprecations, providing clear guidance towards the new API. This consistency is beneficial for maintainability.packages/language/core/v1/src/next/applicables/canDeposit.ts (1)
- 7-19: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [10-27]
The deprecation of
CanDepositand related functions is handled consistently with other deprecations, providing clear guidance towards the new API. This consistency is beneficial for maintainability.packages/language/core/v1/src/next/index.ts (1)
- 11-30: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-39]
The deprecation of the
Nexttype and related functions is clearly communicated, guiding developers towards the new API. This module-wide deprecation aligns with the PR's objectives and ensures backward compatibility while encouraging the use of the new API.
| /** | ||
| * Converts an {@link ApplicableAction} into an {@link ApplicableInput}. | ||
| * | ||
| * This function has two {@link https://www.tutorialsteacher.com/typescript/function-overloading | overloads}. This | ||
| * one can be used with {@link @marlowe.io/language-core-v1!index.Notify}, | ||
| * {@link @marlowe.io/language-core-v1!index.Deposit} actions, or to Advance a timed out | ||
| * contract, none of which require further information. | ||
| */ |
There was a problem hiding this comment.
The documentation for the getInput method's overloads could be enhanced for clarity. While the existing comment mentions the concept of function overloading and its applicability to certain action types, it could benefit from more detailed examples or a direct link to TypeScript's documentation on function overloading. This would aid users unfamiliar with the concept and provide a clearer understanding of how to use the overloads effectively.
Consider enhancing the documentation comment with more examples or a direct link to TypeScript's documentation on function overloading.
| if (!chosenNum) { | ||
| throw new Error("Chosen number is not provided"); | ||
| } | ||
| if (!inBounds(chosenNum, choice.choose_between)) { | ||
| throw new Error("Chosen number is not in bounds"); |
There was a problem hiding this comment.
The error handling for the Choice action type in the doMakeApplicableInput function could be more descriptive. The current error messages are generic and might not provide enough context for developers to understand the issue clearly.
- throw new Error("Chosen number is not provided");
+ throw new Error("Chosen number is required but was not provided for the Choice action.");
- throw new Error("Chosen number is not in bounds");
+ throw new Error(`Chosen number ${chosenNum} is not within the specified bounds for the Choice action.`);Improving the error messages as suggested would help developers diagnose issues more efficiently.
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.
| if (!chosenNum) { | |
| throw new Error("Chosen number is not provided"); | |
| } | |
| if (!inBounds(chosenNum, choice.choose_between)) { | |
| throw new Error("Chosen number is not in bounds"); | |
| if (!chosenNum) { | |
| throw new Error("Chosen number is required but was not provided for the Choice action."); | |
| } | |
| if (!inBounds(chosenNum, choice.choose_between)) { | |
| throw new Error(`Chosen number ${chosenNum} is not within the specified bounds for the Choice action.`); |
| ); | ||
| if ("transaction_error" in txOut) { | ||
| // TODO: Improve error | ||
| throw new Error("There was a transaction error" + txOut.transaction_error); |
There was a problem hiding this comment.
The error message in the simulateApplicableInput function could be improved by providing more context. When a transaction error occurs, including details about the input or contract state that led to the error could significantly aid in debugging.
Consider enhancing the error message to include more context, such as the input or contract details, to aid in debugging.
| ): GetApplicableActionsDI => { | ||
| return { | ||
| getContractContinuation: (contractSourceId: ContractSourceId) => { | ||
| // TODO: Add caching |
There was a problem hiding this comment.
The TODO comment about adding caching to the getContractContinuation function highlights an important consideration for performance. Implementing caching could significantly reduce the number of network requests, improving the overall efficiency of the application.
Would you like me to propose a caching strategy or implementation for this function?
This PR adds a new API to the runtimeLifecycle package that replaces the current
NextAPI.The responsibilities of this API is to compute what are the next possible applicable actions of a contract, get the inputs out of those actions, simulate the results and apply them to the blockchain.
I'm calling this the first version of the new API as it also partially introduces a new ContractsAPI that has more semantic information regarding the contract, and this API might change once the new ContractsAPI is more mature.
To see how this is used, check the
marlowe-object-flowexample.After this API settles, we should also modify the runtimes WEB API to make this functionality available in projects that don't use the TS-SDK.
Summary by CodeRabbit
Summary by CodeRabbit
marlowe-objectand Marlowe Lifecycle package documentation.