feat(udt): add udt info querying methods
For full context see #228
🦋 Changeset detected
Latest commit: a242ce71ba313c501af17894fe64d7a8c57dca8b
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 20 packages
| Name | Type |
|---|---|
| @ckb-ccc/ssri | Minor |
| @ckb-ccc/udt | Minor |
| @ckb-ccc/core | Patch |
| @ckb-ccc/shell | Patch |
| @ckb-ccc/eip6963 | Patch |
| @ckb-ccc/joy-id | Patch |
| @ckb-ccc/lumos-patches | Patch |
| @ckb-ccc/nip07 | Patch |
| @ckb-ccc/okx | Patch |
| @ckb-ccc/rei | Patch |
| @ckb-ccc/spore | Patch |
| @ckb-ccc/uni-sat | Patch |
| @ckb-ccc/utxo-global | Patch |
| @ckb-ccc/xverse | Patch |
| @ckb-ccc/ccc | Patch |
| ckb-ccc | Patch |
| @ckb-ccc/connector | Patch |
| @ckb-ccc/examples | Patch |
| @ckb-ccc/ccc-playground | Patch |
| @ckb-ccc/connector-react | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
@phroi This should allow us to query info about UDT-like cells. One issue left: How should we handle the custom UDT cell while changing?
https://github.com/ckb-devrel/ccc/blob/439380360a053c511c5ef741b3afeca77da9096a/packages/udt/src/udt/index.ts#L1368-L1371 and https://github.com/ckb-devrel/ccc/blob/439380360a053c511c5ef741b3afeca77da9096a/packages/udt/src/udt/index.ts#L1435
Deploy Preview for apiccc ready!
| Name | Link |
|---|---|
| Latest commit | a242ce71ba313c501af17894fe64d7a8c57dca8b |
| Latest deploy log | https://app.netlify.com/projects/apiccc/deploys/68a22d0a0f45000008e8a76d |
| Deploy Preview | https://deploy-preview-261--apiccc.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
Lighthouse |
1 paths audited Performance: 94 (🟢 up 12 from production) Accessibility: 90 (no change from production) Best Practices: 92 (no change from production) SEO: 83 (no change from production) PWA: - View the detailed breakdown and full score reports |
To edit notification comments on pull requests, go to your Netlify project configuration.
Deploy Preview for appccc ready!
| Name | Link |
|---|---|
| Latest commit | a242ce71ba313c501af17894fe64d7a8c57dca8b |
| Latest deploy log | https://app.netlify.com/projects/appccc/deploys/68a22d0af509cb00085828c1 |
| Deploy Preview | https://deploy-preview-261--appccc.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
Lighthouse |
1 paths audited Performance: 53 (🔴 down 16 from production) Accessibility: 88 (no change from production) Best Practices: 92 (🔴 down 8 from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports |
To edit notification comments on pull requests, go to your Netlify project configuration.
Deploy Preview for docsccc ready!
| Name | Link |
|---|---|
| Latest commit | a242ce71ba313c501af17894fe64d7a8c57dca8b |
| Latest deploy log | https://app.netlify.com/projects/docsccc/deploys/68a22d0a5766880008f4b046 |
| Deploy Preview | https://deploy-preview-261--docsccc.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
Lighthouse |
1 paths audited Performance: 70 (🟢 up 6 from production) Accessibility: 88 (no change from production) Best Practices: 92 (no change from production) SEO: 92 (no change from production) PWA: - View the detailed breakdown and full score reports |
To edit notification comments on pull requests, go to your Netlify project configuration.
Deploy Preview for liveccc ready!
| Name | Link |
|---|---|
| Latest commit | a242ce71ba313c501af17894fe64d7a8c57dca8b |
| Latest deploy log | https://app.netlify.com/projects/liveccc/deploys/68a22d0a83f3600008543ad1 |
| Deploy Preview | https://deploy-preview-261--liveccc.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
Lighthouse |
1 paths audited Performance: 9 (no change from production) Accessibility: 88 (no change from production) Best Practices: 92 (🔴 down 8 from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports |
To edit notification comments on pull requests, go to your Netlify project configuration.
/gemini review
This is phase 2 of the UDT master plan:
- Try to add more querying methods, like calculating info of a Signer. Meanwhile, we should have methods for parsing a single UDT cell. On this stage, we should consider if these new methods can be overridden to accommodate some unconventional UDT cell requirements.
Back to your comment:
This should allow us to query info about UDT-like cells. One issue left: How should we handle the custom UDT cell while changing?
You meant while chaining, right?
Yeah, I have feelings about those two methods!! :rofl:
I don't particularly like static balanceFrom: notice how I can call that method on any cell, irregarding of how it may be a UDT or not. It will also cause confusion if actually is called on cells that seems like UDTs but actually are not (iCKB Receipts)
Additionally, I don't particularly like the non-static balanceFrom:
- At the very least we need to check the type of the cell against
UDT.script - Also, for subclasses compatibility reasons, you need to pass the outpoint (if exists) and a ccc.Client parameter (headers of iCKB Receipts).
- At that point you realize that not all protocols are chainable and that's a fact of life.
For example in the case of iCKB Receipts:
- iCKB Receipts as output do have a zero iCKB value (cause we don't know the transaction header)
- iCKB Receipts as inputs have a positive iCKB value (cause we know the transaction header)
Similarly goes for iCKB Deposits:
- iCKB Deposits as output do have a zero iCKB value (cause we don't know the transaction) header.
- iCKB Deposits as inputs have a negative iCKB value (cause we know the transaction header)
As outputs (so when depositing), the iCKB Receipt will just track the value of the free CKB of the iCKB Deposit(s). This way the protocol work around the on-chain determinism of L1, which doesn't allow to read the transaction inclusion header at validation time.
As inputs (so when withdrawing), the positive Receipt iCKB value (and iCKB UDTs) and negative Deposit iCKB value cancel each other out.
Are we able to move somewhat closer to infoFrom?
Phroi %36
Just one thing that makes me wonder is filter: correct if we handle only UDT, but we'll be able to extend it later?
I know that it's tightly coupled with tx.completeInputs
Maybe it's not necessary for it to be extendable, cause maybe we don't want to automatically add non-UDT cells
Maybe a second filter or something for other cells types will be needed by subclasses and that's about it
I'm unsure about the scenario here. You mean maybe we will need multiple filters? Or may we need more criteria in the filter?
If it's the first one, will having multiple Udt instances be better? If it's the second one, the filter is defined by the CKB's PRC. It will be hard to make it extendable.
First case. I'm very much aware of CKB RPC methods and parameters.
Take for example iCKB, a user may own both UDTs and iCKB Receipt cells, which have types that are completely different from each-other, but for example both cells needs to be accounted in the same UDT value for a transaction.
Phroi %11
This should allow us to query info about UDT-like cells. One issue left: How should we handle the custom UDT cell while changing?
You meant
while chaining, right?
Nah, I mean while giving the change cell. I thought the change could be used as a verb.
So, for example, we're dealing with an iCKB receipt. How do we construct the change cell so it's a valid cell?
I don't particularly like
static balanceFrom: notice how I can call that method on any cell, irregarding of how it may be a UDT or not. It will also cause confusion if actually is called on cells that seems like UDTs but actually are not (iCKB Receipts)
It's for convenience, so devs won't need to make a Udt instance to parse the balance. What about giving it a more detailed name e.g. balanceFromUnsafe or sth?
Additionally, I don't particularly like the non-static
balanceFrom:
- At the very least we need to check the type of the cell against
UDT.script
Sure. If it's not an expected UDT cell, let's return 0.
- Also, for subclasses compatibility reasons, you need to pass the outpoint (if exists) and a ccc.Client parameter (headers of iCKB Receipts).
Of course. Do you think any more info is needed?
- At that point you realize that not all protocols are chainable and that's a fact of life.
Absolutely. We just want to try our best to make things faster.
Are we able to move somewhat closer to
infoFrom?
I'm unsure in what perspective we can make them closer.
- I can accept having an
infoFromsimilar to the currentbalanceFrom, but returns more detail. I'll still keep thebalanceFrombut invoke theinfoFrominside it. - I can accept letting the method accept multiple cells and accumulate their info (need to design the signature so it's still clear with only one cell);
- I can accept moving the
initialInfoto the last parameter and making it optional;
Is it good enough for you?
So, for example, we're dealing with an iCKB receipt. How do we construct the change cell so it's a valid cell?
This still makes me struggle, and infoFrom doesn't seem to solve it. Looking forward to your suggestions!
Take for example iCKB, a user may own both UDTs and iCKB Receipt cells, which have types that are completely different from each-other, but for example both cells needs to be accounted in the same UDT value for a transaction.
I have two possible solutions:
- Making the
filteran array. But I would like to make itprivateuntil we see cases really need to access theUdt.filters, since I'm not sure if we will still change that in the future. -
iCkbUdtcan override thecompleteInputsmethod.
- Also, for subclasses compatibility reasons, you need to pass the outpoint (if exists) and a ccc.Client parameter (headers of iCKB Receipts).
Of course. Do you think any more info is needed?
That also means balanceFrom/infoFrom needs to be async, right?
Nah, I mean while giving the change cell. I thought the change could be used as a verb.
You are indeed correct, it is a verb, my bad! I just didn't get the meaning, I'm so sorry!! :rofl:
So, for example, we're dealing with an iCKB receipt. How do we construct the change cell so it's a valid cell?
You just add a UDT cell as usual, if for any reason the transaction has a positive burned amount of iCKB.
Change cell will be always an UDT, even in iCKB. Receipt only tracks deposits in output, so in a way is a kind of change cell in itself.
The iCKB validation at L1 script level is:
in_udt_ickb + in_receipts_ickb == out_udt_ickb + in_deposits_ickb
It's for convenience, so devs won't need to make a Udt instance to parse the balance. What about giving it a more detailed name e.g. balanceFromUnsafe or sth?
Something like that, with the appropriate warning in the TypeDoc
Also, for subclasses compatibility reasons, you need to pass the outpoint (if exists) and a ccc.Client parameter (headers of iCKB Receipts).
Of course. Do you think any more info is needed?
Another thought that comes up sometimes is that at time two or more cells cells need to be consumed togheter. That said, usually there is one cell holding the value, while the other controls the logic of accessing it, so should be fine as it is.
That also means balanceFrom/infoFrom needs to be async, right?
Yesss, think for example if these methods are implemented in SSRI.
Are we able to move somewhat closer to https://github.com/ckb-devrel/ccc/pull/250?
I'm unsure in what perspective we can make them closer.
Just async + client + outpoint should be enough
I have two possible solutions:
- Making the filter an array. But I would like to make it private until we see cases really need to access the Udt.filters, since I'm not sure if we will still change that in the future.
Array has an issue: different scripts may have different uses and purposes, as with iCKB Receipt. If it was a named tuple it would make sense for sub-classes, but not for base.
For example, an issue is that consuming each of these cells could be done in a different way, we cannot tell for sure how the Transaction will be modified by consuming a non-UDT cell. iCKB Receipts need header in HeaderDeps, for example.
- iCkbUdt can override the completeInputs method.
And all methods using directly UDT.filter, like calculateInfo.
- I just ovverride in iCKB UDT the minimal methods like
balanceFrom. Dev/user will need to manually add these non-UDT cells, possibly they will not be counted bycalculateInfo.
Phroi %38
So, for example, we're dealing with an iCKB receipt. How do we construct the change cell so it's a valid cell?
You just add a UDT cell as usual, if for any reason the transaction has a positive burned amount of iCKB.
Change cell will be always an UDT, even in iCKB. Receipt only tracks deposits in output, so in a way is a kind of change cell in itself.
The iCKB validation at L1 script level is:
in_udt_ickb + in_receipts_ickb == out_udt_ickb + in_deposits_ickb
Glad to hear that we don't need more detailed control currently! This should make things easier.
I have two possible solutions:
- Making the filter an array. But I would like to make it private until we see cases really need to access the Udt.filters, since I'm not sure if we will still change that in the future.
Array has an issue: different scripts may have different uses and purposes, as with iCKB Receipt. If it was a named tuple it would make sense for sub-classes, but not for base.
- iCkbUdt can override the completeInputs method.
And all methods using directly
UDT.filter, likecalculateInfo.
Seems like there is no easy solution for this. Perhaps we could create two Udt subclasses, UdtICkb and UdtICkbReceipt, with overridden isUdt methods. This would allow them to recognize others' balances during transaction composition. But when fetching cells on-chain, they only handle their own script.
Seems like there is no easy solution for this. Perhaps we could create two Udt subclasses, UdtICkb and UdtICkbReceipt, with overridden isUdt methods. This would allow them to recognize others' balances during transaction composition. But when fetching cells on-chain, they only handle their own script.
I currently have something along those lines, I need to check the feasibility of Subclassing, but I'm honestly too sleepy now...
If you want to try your hand with a commit, as soon as I wake up I'll check the feasibility!
Wish you a good sleep, night night :hugs:
Phroi
/gemini review
/gemini review
#262 should be reviewed first.
Now let's go back to here @phroi
Do you think the new infoFrom design works well with iCKB?
Do you think the new infoFrom design works well with iCKB?
Issue with iCKB is that the value of a cell varies if it's used as input or output of a transaction.
With CellAny we are not able to distinguish that, we only know if the outpoint is given or not
Which is different from if a cell is used as input or output
Seems like I had the same issue in my own code
infoFrom may not be a good idea after all 🤷♂️
Similar issue goes with the capacity of NervosDAO cells
Do you think the new infoFrom design works well with iCKB?
Issue with iCKB is that the value of a cell varies if it's used as input or output of a transaction.
With CellAny we are not able to distinguish that, we only know if the outpoint is given or not
Which is different from if a cell is used as input or output
Seems like I had the same issue in my own code
I suggest that we can safely assume all CellAny with outPoint are inputs, and outputs if without.
What if someone is fetching old transaction and he keeps both inputs and outputs as ccc.Cell to re-validate what happened in that transaction?
Having only the oupoint as un/defined to distinguish between inputs and outputs use is finicky and fragile
Yesterday I was thinking, what if we remove inforFrom and to determine values of single cells we just add the single cell to an empty transaction and from there getInputInfo/getOutputInfo
Using getInputInfo/getOutputInfo then just boils down to expand appropriately the methods interface to include non-tx types?
Alternatively, we need to explicitly pass the intended use input/output to infoFrom
What if someone is fetching old transaction and he keeps both inputs and outputs as ccc.Cell to re-validate what happened in that transaction?
That means, the dev gives up getInpusInfo & getOutputsInfo, chooses to manually construct output Cells, and passes them into infoFrom, just to get a wrong result.
I would say that won't be our problem.
Alternatively, we need to explicitly pass the intended use input/output to infoFrom
That seems to make things more semantically correct. But we can also assume "cells with outpoints are inputs".
As I mentioned, if we also assign outPoint to outputs, things will be much more painful. If not, we can do the assumption.
Ok, as reviewer I had to raise the concern! Assumption is reasonable, could you also add some remarks in the TypeDoc?
I was wondering, have you considered passing acc/origin as parameter to infoFrom?
