PLT-7693 get transaction by id #42

hrajchert · 2023-10-16T13:37:00Z

This PR:

Adds the runtime-rest-client endpoint: getTransactionById
Improves the generated documentation of related runtime-rest-client endpoint
Simplifies rest-client-flow.html to avoid repetition and add error handling

hrajchert · 2023-10-16T13:48:24Z

packages/adapter/src/io-ts.ts

+export function proxy<A = never>(): A {
+  return null as any;
+}


After making this I realized that I should probably try to make a Proxy<T> type that is unusable instead of casting to A. For the purpose that is being used it shouldn't matter, but it would be better of course

I'm assuming this is missing functionality in fp-ts and io-ts and that it corresponds to data Proxy (t :: k) from Haskell.

Can we live without this? It looks like growing tech debt. Is there an alternate solution where we cut out some fp-ts code instead which makes this unecessary?

It is not missing functionality from fp-ts/io-ts in the sense that this is not required for "normal use". It is required by us for the reason I wrote at the beginning of the file.

If you use TypeScript (or any statically typed language) you'll eventually need some sort of dynamic checks or you are bound to have runtime errors.

For TS as far as I know you have:

io-ts

runtypes

zod

and possible others...

Back in the day I created parmenides with a friend before the other libraries existed/got traction.

Knowing that we need runtime validation, and we already have io-ts in place (which has all the features we need and more), I don't believe this is adding to the technical debt. We just need to be judicious on what and how we expose things. See this exported doc as an example and note the difference between the types and the dynamic guards.

hrajchert · 2023-10-16T13:58:40Z

packages/runtime/client/rest/src/contract/endpoints/collection.ts

+   * @see {@link @marlowe.io/wallet!api.WalletAPI#signTx}
+   * @see {@link index.RestAPI#submitContract}
+   */
+  // QUESTION: Should we rename the property or the type to indicate that is unsigned?


cc: @jhbertra @nhenin

Yes we should imo

cc: @jhbertra if you agree we can schedule this for 0.0.6

hrajchert · 2023-10-16T13:59:43Z

packages/runtime/client/rest/src/contract/transaction/details.ts

+ * @experimental
+ * This type is experimental and subject to change. See current DISCUSSIONS in the source code.
+ */


cc @nhenin , we should probably add this note to any experimental endpoint or attribute

hrajchert · 2023-10-16T14:00:21Z

packages/runtime/client/rest/src/contract/transaction/details.ts

+  // DISCUSSION: @Jamie, @N.H. The optional part is because this is only present if the transaction is confirmed?
+  //             If so, we should maybe include the Block Header as part of the confirmed constructor to avoid
+  //             impossible states?
+  block: O.Option<BlockHeader>;
+  /**
+   * The UTXO that was used as input for the transaction.
+   */
+  inputUtxo: TxOutRef;
+  /**
+   * The list of Marlowe inputs that were applied in this transaction
+   */
+  inputs: Input[];
+  // DISCUSSION: @Jamie, @N.H. Similar discussion as the block one. Are these Options part of a confirmed
+  //            transaction?
+  //            Looking for this contractid and txid it has None for these fields in preprod
+  //            contractID "19a90f8db56fab54d5e4fc4a4c0fd5084bb280a07841f2182b22305962285ad1#1",
+  //            txId "b733caef6a21526b98586112d2090d0e1373654bf17ad85240fb12f08fb58802"
+  outputUtxo: O.Option<TxOutRef>;
+  outputContract: O.Option<Contract>;
+  outputState: O.Option<MarloweState>;
+  // QUESTION: @Jamie, @N.H, what is this?
+  consumingTx: O.Option<TxId>;
+  // DISCUSSION: @Jamie, @N.H. Should this be a TimeInterval instead?
+  invalidBefore: ISO8601;
+  invalidHereafter: ISO8601;
+  // QUESTION: @Jamie, what is this? The signed txbody?
+  txBody: O.Option<TextEnvelope>;


cc @nhenin @jhbertra

there are some states behind these Options that are not explicit... I would prefer have a notion of state instead of these pieces of information flatten...

hrajchert · 2023-10-16T14:00:42Z

packages/runtime/client/rest/src/index.ts

 */
+// DISCUSSION: @N.H: Should we rename this to RestClient?


Yes we should

nhenin · 2023-10-19T08:55:32Z

packages/runtime/client/rest/src/contract/endpoints/collection.ts

+   * @see {@link @marlowe.io/wallet!api.WalletAPI#signTx}
+   * @see {@link index.RestAPI#submitContract}
+   */
+  // QUESTION: Should we rename the property or the type to indicate that is unsigned?


Yes we should imo

nhenin · 2023-10-19T08:59:41Z

packages/runtime/client/rest/src/contract/transaction/details.ts

+  // DISCUSSION: @Jamie, @N.H. The optional part is because this is only present if the transaction is confirmed?
+  //             If so, we should maybe include the Block Header as part of the confirmed constructor to avoid
+  //             impossible states?
+  block: O.Option<BlockHeader>;
+  /**
+   * The UTXO that was used as input for the transaction.
+   */
+  inputUtxo: TxOutRef;
+  /**
+   * The list of Marlowe inputs that were applied in this transaction
+   */
+  inputs: Input[];
+  // DISCUSSION: @Jamie, @N.H. Similar discussion as the block one. Are these Options part of a confirmed
+  //            transaction?
+  //            Looking for this contractid and txid it has None for these fields in preprod
+  //            contractID "19a90f8db56fab54d5e4fc4a4c0fd5084bb280a07841f2182b22305962285ad1#1",
+  //            txId "b733caef6a21526b98586112d2090d0e1373654bf17ad85240fb12f08fb58802"
+  outputUtxo: O.Option<TxOutRef>;
+  outputContract: O.Option<Contract>;
+  outputState: O.Option<MarloweState>;
+  // QUESTION: @Jamie, @N.H, what is this?
+  consumingTx: O.Option<TxId>;
+  // DISCUSSION: @Jamie, @N.H. Should this be a TimeInterval instead?
+  invalidBefore: ISO8601;
+  invalidHereafter: ISO8601;
+  // QUESTION: @Jamie, what is this? The signed txbody?
+  txBody: O.Option<TextEnvelope>;


there are some states behind these Options that are not explicit... I would prefer have a notion of state instead of these pieces of information flatten...

nhenin · 2023-10-19T09:00:00Z

packages/runtime/client/rest/src/contract/transaction/details.ts

+  // DISCUSSION: @Jamie, @N.H. The optional part is because this is only present if the transaction is confirmed?
+  //             If so, we should maybe include the Block Header as part of the confirmed constructor to avoid
+  //             impossible states?
+  block: O.Option<BlockHeader>;
+  /**
+   * The UTXO that was used as input for the transaction.
+   */
+  inputUtxo: TxOutRef;
+  /**
+   * The list of Marlowe inputs that were applied in this transaction
+   */
+  inputs: Input[];
+  // DISCUSSION: @Jamie, @N.H. Similar discussion as the block one. Are these Options part of a confirmed
+  //            transaction?
+  //            Looking for this contractid and txid it has None for these fields in preprod
+  //            contractID "19a90f8db56fab54d5e4fc4a4c0fd5084bb280a07841f2182b22305962285ad1#1",
+  //            txId "b733caef6a21526b98586112d2090d0e1373654bf17ad85240fb12f08fb58802"
+  outputUtxo: O.Option<TxOutRef>;
+  outputContract: O.Option<Contract>;
+  outputState: O.Option<MarloweState>;
+  // QUESTION: @Jamie, @N.H, what is this?
+  consumingTx: O.Option<TxId>;
+  // DISCUSSION: @Jamie, @N.H. Should this be a TimeInterval instead?
+  invalidBefore: ISO8601;
+  invalidHereafter: ISO8601;
+  // QUESTION: @Jamie, what is this? The signed txbody?
+  txBody: O.Option<TextEnvelope>;


nhenin · 2023-10-19T09:01:03Z

packages/runtime/client/rest/src/index.ts

 */
+// DISCUSSION: @N.H: Should we rename this to RestClient?


Yes we should

nhenin · 2023-10-19T09:04:16Z

@hrajchert can you resolve the conflicts ? and we can merge it

hrajchert changed the title ~~PLT-7693 get transaction by~~ PLT-7693 get transaction by id Oct 16, 2023

hrajchert added the No Changelog Required label Oct 16, 2023

hrajchert commented Oct 16, 2023

View reviewed changes

hrajchert requested review from nhenin and jhbertra October 16, 2023 14:04

hrajchert mentioned this pull request Oct 18, 2023

PLT-7692: Create flat version of createTransactionForContract #43

Merged

nhenin approved these changes Oct 19, 2023

View reviewed changes

hrajchert and others added 9 commits October 19, 2023 11:09

separated ContractHeaderGuard from the type

ddc8116

separated GetContractsResponse from the type

83f6e64

Fix type assertion of branded and newtype guards

84ddb40

Add Get contract transaction by id to the runtime-rest-client

9fd4a4f

Improved DX around createContract

8eac6aa

Simplify Rest Client Flow

422cfbf

Improve documentation

f39e851

Rollback rest-client-flow script importmap

54ebc32

treefmt

0894555

bjornkihlberg force-pushed the hrajchert/PLT-7693-get-transaction-by-id branch from a7c30b5 to 0894555 Compare October 19, 2023 09:14

nhenin merged commit 0d881ef into main Oct 19, 2023
1 check passed

nhenin deleted the hrajchert/PLT-7693-get-transaction-by-id branch October 19, 2023 09:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PLT-7693 get transaction by id #42

PLT-7693 get transaction by id #42

hrajchert commented Oct 16, 2023

hrajchert Oct 16, 2023

bjornkihlberg Oct 17, 2023

hrajchert Oct 17, 2023

hrajchert Oct 16, 2023

nhenin Oct 19, 2023

hrajchert Oct 19, 2023

hrajchert Oct 16, 2023

hrajchert Oct 16, 2023

nhenin Oct 19, 2023

nhenin Oct 19, 2023

hrajchert Oct 16, 2023

nhenin Oct 19, 2023

nhenin Oct 19, 2023

nhenin Oct 19, 2023

nhenin Oct 19, 2023

nhenin Oct 19, 2023

nhenin commented Oct 19, 2023

PLT-7693 get transaction by id #42

PLT-7693 get transaction by id #42

Conversation

hrajchert commented Oct 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhenin commented Oct 19, 2023