ccc icon indicating copy to clipboard operation
ccc copied to clipboard

feat(udt): add udt info querying methods

Open Hanssen0 opened this issue 5 months ago • 42 comments

For full context see #228

Hanssen0 avatar Aug 16 '25 19:08 Hanssen0

🦋 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

changeset-bot[bot] avatar Aug 16 '25 19:08 changeset-bot[bot]

@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

Hanssen0 avatar Aug 16 '25 19:08 Hanssen0

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...

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.

netlify[bot] avatar Aug 16 '25 19:08 netlify[bot]

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...

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.

netlify[bot] avatar Aug 16 '25 19:08 netlify[bot]

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...

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.

netlify[bot] avatar Aug 16 '25 19:08 netlify[bot]

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...

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.

netlify[bot] avatar Aug 16 '25 19:08 netlify[bot]

/gemini review

Hanssen0 avatar Aug 16 '25 19:08 Hanssen0

This is phase 2 of the UDT master plan:

  1. 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

phroi avatar Aug 16 '25 20:08 phroi

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

phroi avatar Aug 16 '25 20:08 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?

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.

  1. I can accept having an infoFrom similar to the current balanceFrom, but returns more detail. I'll still keep the balanceFrom but invoke the infoFrom inside it.
  2. 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);
  3. I can accept moving the initialInfo to 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!

Hanssen0 avatar Aug 16 '25 21:08 Hanssen0

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:

  1. 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.
  2. iCkbUdt can override the completeInputs method.

Hanssen0 avatar Aug 16 '25 21:08 Hanssen0

  • 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?

Hanssen0 avatar Aug 16 '25 21:08 Hanssen0

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:

  1. 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.

  1. iCkbUdt can override the completeInputs method.

And all methods using directly UDT.filter, like calculateInfo.

  1. 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 by calculateInfo.

Phroi %38

phroi avatar Aug 16 '25 21:08 phroi

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:

  1. 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.

  1. iCkbUdt can override the completeInputs method.

And all methods using directly UDT.filter, like calculateInfo.

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.

Hanssen0 avatar Aug 16 '25 21:08 Hanssen0

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

phroi avatar Aug 16 '25 22:08 phroi

/gemini review

Hanssen0 avatar Aug 17 '25 01:08 Hanssen0

/gemini review

Hanssen0 avatar Aug 17 '25 01:08 Hanssen0

#262 should be reviewed first.

Hanssen0 avatar Aug 17 '25 01:08 Hanssen0

Now let's go back to here @phroi Do you think the new infoFrom design works well with iCKB?

Hanssen0 avatar Aug 17 '25 15:08 Hanssen0

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

phroi avatar Aug 17 '25 15:08 phroi

infoFrom may not be a good idea after all 🤷‍♂️

phroi avatar Aug 17 '25 15:08 phroi

Similar issue goes with the capacity of NervosDAO cells

phroi avatar Aug 17 '25 15:08 phroi

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.

Hanssen0 avatar Aug 17 '25 15:08 Hanssen0

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?

phroi avatar Aug 17 '25 15:08 phroi

Having only the oupoint as un/defined to distinguish between inputs and outputs use is finicky and fragile

phroi avatar Aug 17 '25 15:08 phroi

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

phroi avatar Aug 17 '25 15:08 phroi

Using getInputInfo/getOutputInfo then just boils down to expand appropriately the methods interface to include non-tx types?

phroi avatar Aug 17 '25 15:08 phroi

Alternatively, we need to explicitly pass the intended use input/output to infoFrom

phroi avatar Aug 17 '25 15:08 phroi

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.

Hanssen0 avatar Aug 17 '25 15:08 Hanssen0

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?

phroi avatar Aug 17 '25 16:08 phroi