PLT-7691 flat version get transaction per contract #40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hrajchert
commented
Oct 11, 2023
Comment on lines
+96
to
+98
| // DISCUSSION: What should this return when contractId is not found? Currently it throws an exception | ||
| // with an AxiosError 404, we could return a nullable value or wrap the error into a custom | ||
| // ContractNotFound error and specify it in the docs. |
There was a problem hiding this comment.
It's fine if we throw an exception here - the caller should be sure the contract exists, not use this as a means of testing for its existence. Wrapping in a custom error is not a bad idea.
hrajchert
commented
Oct 11, 2023
| @@ -5,7 +5,7 @@ import { split } from "fp-ts/lib/string.js"; | |||
| import { pipe } from "fp-ts/lib/function.js"; | |||
| import { head } from "fp-ts/lib/ReadonlyNonEmptyArray.js"; | |||
| import { TxId } from "../tx/id.js"; | |||
| import { ContractId } from "../contract/id.js"; | |||
| import { ContractIdGuard } from "../contract/id.js"; | |||
| import { AssetId, Assets } from "../asset/index.js"; | |||
|
|
|||
| // QUESTION: @N.H: What is the difference between PayoutId and WithdrawalId? | |||
There was a problem hiding this comment.
PayoutId is the id of the payout 🙂 meaning you’ll use it to withdraw the tokens from these payouts….
WithdrawalId are the id of the Tx when you withdraw from this payout…
hrajchert
commented
Oct 11, 2023
| @@ -3,10 +3,18 @@ import * as t from "io-ts/lib/index.js"; | |||
| export type Tag = t.TypeOf<typeof Tag>; | |||
| export const Tag = t.string; | |||
|
|
|||
| export type TagContent = t.TypeOf<typeof TagContent>; | |||
| // QUESTION: @Jamie, N.H. is this right, TagContent can be anything? | |||
nhenin
reviewed
Oct 12, 2023
| @@ -5,7 +5,11 @@ | |||
| <meta http-equiv="X-UA-Compatible" content="IE=edge" /> | |||
| <title>Vesting Flow</title> | |||
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | |||
| <script src="/dist/local-importmap.js"></script> | |||
| <!-- This import map tells the browser what version of the SDK to use, use the local-importmap for local | |||
| development and the jsdelivr importmap for the latest published version. | |||
nhenin
approved these changes
Oct 12, 2023
nhenin
deleted the
hrajchert/PLT-7691-flat-version-get-transaction-per-contract
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the method
getTransactionsForContractto the RestAPI of theruntime-rest-clientand improves the generated documentation of related methods. To test you can build the project, modify thepoc/rest-client-flow.htmlso that it uses local build and not latest release and then use the new method from the page.Also, build the docs and make sure that the documentation makes sense.
@nhenin , @jhbertra , please note the DISCUSSION topics added in this PR