feat(udt): balance related util methods

[Hanssen0]

For #227

[changeset-bot]

🦋 Changeset detected

Latest commit: 2eca243

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@ckb-ccc/ssri	Minor
@ckb-ccc/udt	Minor
@ckb-ccc/shell	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]

Replying to the comment on the issue (and on the code):

We'll make it async.

I'm reading thru the PR and I see you were thinking about deprecating those Balance methods on Transaction, which in a way indeed solves the async issue. (Nice new TypeDocs by the way!! 🥳)

Generally speaking, in Transaction you have no sure way of calculating those as UDT over time is gonna have more and more implementations corner cases. For example, iCKB UDT is one of these corner cases, since there are cells which are not UDT, but still hold UDT value.

SmartTransaction conversely would have a far better shot at it since it holds the UDT Handlers.

it's possible that some Udt in the future needs more than adding cellDeps, maybe add some headerDeps or witnesses.

These days I was also considering letting iCKB UDT subclass also add iCKB Deposit Receipts if needed (as first choice even before actual UDT cells). If I was to do that then I would indeed need to modify headerDeps in completeBy.

Side Note: of course I see a future where we'll move everything to the witness, once CKB is too expensive. Sometimes I wonder if it makes sense calling these utilities udtSomething cause it will come the time when tokens are expressed differently on-chain, but the CCC interfaces we are defining now will still be applicable. This is the reason why I remember myself that UDT just stands for User Defined Token and as standards evolve its meaning could change.

We added a feature similar to this last week with a suggestion from @rink1969 #216
And I improved the algorithm in #228.

Nice, I see that you refined it similarly to the way I coded it, so by calculating the capacity of these cells, not by counting them 👍

Just, if I can add something in this regard, I'd say that you should segregate the concerns in the implementation so that sub-classes can reuse completeX code as much as possible by overriding said UDT methods.

[phroi]

Why not using CellLike?

[Hanssen0]

It's more like a CellOnchain, which contains OutPoint of the cell, and it's not necessary for checking. I think it's meaningless to add CellOffchain, so output + data is used here.

[phroi]

You make a fair point, it's not a given this entities have a Outpoint. In my implementation it did, cause it was only used on on-chain cells

[phroi]

Why not also returning the CKB occupied by said UDT (or UDT-related) cells?

[Hanssen0]

Good idea!

[phroi]

For future SSRI compatibility we should make it async

[phroi]

I would call it getBalanceDelta, cause this delta may indeed be positive or negative

[phroi]

Also if we add the CKB to the input and output methods, also this one should return its delta

[Hanssen0]

Once in a while, I name it balanceDiff. I think delta has the same problem as diff, in that we can't intuitively understand whether it represents input minus output or vice versa.

[phroi]

Delta usually is defined Output minus Input, differently from diff, which has no such connotation. That said, Burned is surely more colorful and probably intuitive, maybe it'll cause also more raised eyebrows, but nothing a good TypeDoc can't fix

[phroi]

This is difficult to override, it could make sense to partition this in multiple overridable methods

[phroi]

For example it could make sense to have a generator methods that returns said cells wrapped, possibly wrapped into an object where each cell is annotated with its ckb and udt value

[Hanssen0]

I'm not sure about what we want here. Does this mean Udt.completeInputs is hard to override? It might be easier to understand if a possible way of splitting it could be provided.

[phroi]

Yeah, you're right, my underlying issue is with tx.completeInputs: it may be a fine piece of engineering, I don't like it, my bad!! 🤣

[Hanssen0]

Indeed, Transaction.completeInputs has a complicated method signature to maximize its flexibility while reusing code. In most cases, I think we should override the Transaction.findCells.

[phroi]

If we want to collect all UDT cells, I personally think Transaction.completeInputsAll will be better (of course, we can create a wrapper for it in the Udt class).

I'm not sure. While it is in line with the current Transaction methods, I don't particularly like it. To me seems orthogonal to what the other methods offer, so adding as an options seems natural.

What's your reason for making it into a method?

Phroi %51

[Hanssen0]

What's your reason for making it into a method?

Good question. In short, the single responsibility principle and fewer options will make the method more stable.

From the implementation perspective, unlike shouldAddInputs, which is for disabling existing logic, adding compressState requires extra logic in the Udt.complete method.

I also agree that it's orthogonal to methods like completeFee or Udt.complete, but I think that's the reason why we should not couple it with these methods. This adds complexity to the overall system for limited benefit (saving a line of code, or maybe even less, since the extra options also require code).

[Hanssen0]

the single responsibility principle and fewer options will make the method more stable.

Further read: https://understandlegacycode.com/blog/what-is-wrong-with-boolean-parameters/
In-depth discussion of the problems that such parameters may cause.

[Hanssen0]

Added getInputsInfo and getOutputsInfo and wrapped them to get...Balance for convenience.

[phroi]

Further read: https://understandlegacycode.com/blog/what-is-wrong-with-boolean-parameters/
In-depth discussion of the problems that such parameters may cause.

Let's make an example, I don't like findCells vs findCellsOnChain. I would add onChain : bool as I did in my implementations. Your current stance explain that choice.

From the implementation perspective, unlike shouldAddInputs, which is for disabling existing logic, adding compressState requires extra logic in the Udt.complete method.

I see what you mean, in your implementation shouldAddInputs disable the closure, but compressState has no clear implementation, cause you are both accounting and choosing when to stop. This give you flexibility, for certain things, but rigidity to an hypothetical compressState modifier.

[phroi]

Added getInputsInfo and getOutputsInfo and wrapped them to get...Balance for convenience.

Good idea, on one side we have get...Info (used mainly internally and overridable) and on the other get...Balance (which filters out unneeded information for normal use cases), great change!! 👍👍

[Hanssen0]

Let's make an example, I don't like findCells vs findCellsOnChain. I would add onChain : bool as I did in my implementations. Your current stance explain that choice.

Yes. One exception is Client.findTransactions because we want to keep the method design mapped to the RPC calls. I really regret this choice, I mean, look at the method signature!!!

We've been adding more unit tests recently, and splitting methods into smaller units helps us write shorter tests (and makes them easier for others to override).

[Hanssen0]

At this point, I'd say that adding the specific cellDeps as last step inside completeBy seems reasonable. Even more we can make the specific behavior overridable with a hook.

My original plan was to call the hook after each transaction creation method, like mint or transfer. Seems like you think adding them after Udt.complete is fine. Which makes me think: will it be better to add the extra logic to Udt.complete by inheriting?

[phroi]

One exception is Client.findTransactions because we want to keep the method design mapped to the RPC calls. I really regret this choice, I mean, look at the method signature!!!

My eyes are burning!!! 🤣

[phroi]

This little piece of code is efficiently creating more UDT cells out of thin air: while we could easily compress the state by checking the UDT cells capacity flow (similar to the original @rink1969 idea), we instead are checking the global transaction capacity flow.

I'd propose to evaluate:

  const [inUdt, inCkb] = await this.getInputInfo();
  const [outUdt, outCkb] = await this.getOutputInfo();

  const udtBurned = inUdt - outUdt -  ccc.numFrom(balanceTweak ?? 0);// Why not a Num in the first place?
  const ckbBurned =  inCkb - outCkb - ccc.numFrom(capacityTweak ?? 0);// Why not a Num in the first place?

    if (udtBurned >= ccc.Zero && ckbBurned >= ccc.Zero) {
      return 0;
    }

[Hanssen0]

I thought compressing the UDT much is better. E.g., if I have ten UDT cells with 200 CKBs and one 70 CKB cell (for a fee) and want to transfer some CKB to others (while transferring UDT), I can merge cells and extract CKBs.

For a typical UDT transferring transaction composition, we have only one UDT cell in the output when we call Udt.complete, so these two implementations behave the same. However, if a developer were to compose a non-typical transaction, such as transferring UDT and CKB simultaneously, I'm not sure which one is better. Personally, I prefer the current approach, which prioritizes the compression of UDT to obtain more CKB.

[Hanssen0]

I can live with both, there is no difference in complexity or efficiency.

[phroi]

I prefer the current approach, which prioritizes the compression of UDT to obtain more CKB

Could you make a full example of how the code in lines +694 to +702 compress more? 🤔

[Hanssen0]

e.g. If we have ten 100 UDT cells with 140 CKB for each

For a meta transaction A

Inputs: -
Outputs:
  150 UDT in 140 CKB

After Udt.completeBy, we have
Transaction A1

Inputs:
  2 * 100 UDT in 140 CKB
Outputs:
  150 UDT in 140 CKB
  50 UDT in 140 CKB

This is the same for both implementations.

But for Transaction B

Inputs: -
Outputs:
  150 UDT in 140 CKB
  280 CKB

After the current Udt.completeBy, we have
Transaction B1

Inputs:
  4 * 100 UDT in 140 CKB
Outputs:
  150 UDT in 140 CKB
  280 CKB
  250 UDT in 140 CKB

If we only consider the UDT capacity flow, it will be
Transaction B2

Inputs:
  2 * 100 UDT in 140 CKB
Outputs:
  150 UDT in 140 CKB
  280 CKB
  50 UDT in 140 CKB

Obviously, B1 compressed more UDT than B2.

[phroi]

I'm not really sure that it makes much of a difference, cause your current algo already works pretty well

That said, if there is a chance to add more UDT cells, with this change they get added without extra RPC calls (except for the possible extra get_cells call)

[Hanssen0]

It's kinda edge case if devs added some plain CKB cells along with UDT cells. Though I don't know when it will happen, it's a scenario.

[phroi]

For example, I was thinking about NervosDAO withdrawals, like iCKB, and in the same tx the user fan out his UDT holdings

[phroi]

A bit unrealistic tho

[Hanssen0]

Anyway, it won't increase the complexity, so it is worth.

[phroi]

will it be better to add the extra logic to Udt.complete by inheriting?

For example, if we split completeInputs and we split tx.completeInputs into its own internal method and we move the cellDeps hook to its own method, then completeInputs is a general method, that does the following: get initial balance, call internal completeInputs .... and call add cell deps hook.

What's your take on this?

[phroi]

Also, another details, how many usecases outside of UDT we have for ccc.ErrorTransactionInsufficientCoin?

If all the use-cases are exclusively in UDT, why not bring that Error class definition over here and add a proper message formatting so that it can actually be displayed into an user interface?

So using ccc.FixedPontToString(amount, decimals) and mentioning the symbol of the UDT causing the error. Here we are in UDT so it seems appropriate, differently from Core Transaction.

Whats your take on this?

[Hanssen0]

For example, if we split completeInputs and we split tx.completeInputs into its own internal method and we move the cellDeps hook to its own method, then completeInputs is a general method, that does the following: get initial balance, call internal completeInputs .... and call add cell deps hook.

Sounds good to me. You remind me that the current Udt.completeInputs does more things than Transaction.completeInputs. We should add an extra abstract layer here.

[Hanssen0]

If all the use-cases are exclusively in UDT, why not bring that Error class definition over here and add a proper message formatting so that it can actually be displayed into an user interface?

It's reasonable to move the Error to Udt, I'll handle this.

So using ccc.FixedPontToString(amount, decimals) and mentioning the symbol of the UDT causing the error.

Considering the extra cost (SSRI call or Indexer query), this might not. I think we should let the application do this.

[phroi]

Considering the extra cost (SSRI call or Indexer query), this might not.

Reasonable

It's reasonable to move the Error to Udt

Not sure TBH, if you are not willing to add more metadata, it may may make more sense where it is currently.

I guess that I'll consider sub-classing that error to add more metadata in my iCKB subclass 🤔

[phroi]

I wonder, what if instead we make the error a bit more general and optionally let devs specify those details in sub-classes of UDT?

[Hanssen0]

Not sure TBH, if you are not willing to add more metadata, it may may make more sense where it is currently.

After we deprecated the Transaction.completeInputsByUdt, nothing in the Transaction uses ErrorTransactionInsufficientCoin. So it do should be moved.

I wonder, what if instead we make the error a bit more general and optionally let devs specify those details in sub-classes of UDT?

What do you think if we add a reason?: string property to allow richer information? If devs want to add more metadata/properties, they can inherit the Error class.

[Hanssen0]

Since you are the implementer, I was wondering, in which way do you forecast devs will sub-class UDT to support additional cells (that while not being themselves UDTs) have an UDT value?

I have no idea. The original design here is for sUDT-like scripts, so it's "one script - one token". We want to make its code reusable as much as possible, but if this leads to over-engineering, we will not consider it for the time being.

I gave some thoughts about this and I was thinking about keeping the iCKB UDT subclass simple and overriding just addCellDeps and getInputsInfo. (I'll know for sure only when coding tho)

It will still create some trouble as the calculateBalance result will always be lower or equal than the user real UDT worth (which includes iCKB receipts), but should be ok.

What's your take on this?

Maybe overriding the calculateBalance method? I actually don’t recommend combining these two balances for display, as this will cause the user’s balance on the web page and in the wallet to appear inconsistent, which may cause some confusion.

[phroi]

Maybe overriding the calculateBalance method?

If I override calculateBalance then I need to override also completeInputs method to automatically fetch Receipts cells. At that point I need to override that hypothetical ccc.udtBalanceFrom method we just discussed before, cause iCKB Receipts are not UDT cells.

Would you reconsider your previous stance?

[Hanssen0]

hypothetical

I don’t quite know the structure of the iCKB recipient cell. If you can provide some relevant documentation, I might be able to understand it more easily. So what you mean is that the recipe cell contains a portion of the amount that can be converted into iCKB, but this portion of the amount is not stored in the first 16 bytes?

This sounds like it will cause a lot of problems, including how to generate the change cell and what to do if we want to apply the change to an existing cell. I have no idea for now and need more information.

[phroi]

I don’t quite know the structure of the iCKB recipient cell. If you can provide some relevant documentation, I might be able to understand it more easily

Here you go, feel free to ask away any doubt! 🤗

So what you mean is that the recipe cell contains a portion of the amount that can be converted into iCKB, but this portion of the amount is not stored in the first 16 bytes?

That's correct, to be precise, there is no way in which it could be calculated due to the transaction header not being accessible during transaction validation in deposit phase 1. If said header was accessible, then I would directly mint UDTs 😉

how to generate the change cell

Two meanings, so I'll analyze both:

• Deposit Phase 1: Receipt are effectively the change cell that you get when depositing in the iCKB protocol. No need to handle the generation of the Receipt in the UDT subclass.
• Deposit Phase 2: Receipt are added as input, accounted for their iCKB value and the change cell is a normal UDT change cell.

what to do if we want to apply the change to an existing cell

Two meanings, so I'll analyze both:

• Output Receipt (Deposit Phase 1): Receipts are added in output only when making iCKB deposits. No need to handle the generation of the Receipt in the UDT subclass.
• Input Receipt (Deposit Phase 2): Receipts can only be converted to UDT, never updated.

If it helps you wrapping your head around the concept, this is the iCKB Logic contract, the one validating this conversion.

In general, some protocols like iCKB or dCKB need some non-UDT cells need to hold UDT value and there is no way around that, for now. For example: How can you make an NervosDAO deposit that is also an UDT?

Does this help clarifying why cells that are not UDTs can still hold UDT value? Any doubts?

Love & Peace, Phroi %137

[phroi]

Hey @Hanssen0, I started my implementation based on top of the Udt class and I need to give more feedback.

Generally in Transaction.copy consider returning early if input tx === this. You choose, this is an optimization.

Specifically on Udt, the default Udt.filter.outputDataLenRange is [16, "0xffffffff"]. Consider restricting it to accept only the exact UDT data [16, 17]. and implementers of UDTs can always broaden the filter. Reason is that AFAIK xUDTs can be used to store NFTs, we would never want to consume a NFTs by mistake.

Switching to the main topic, I don't know who thought that returning a Promise<ssri.ExecutorResponse<... | undefined>> was a good idea, but I can tell you that it's only gonna cause trouble to implementers.

Not only errors are handled in different ways depending on the method (throw vs return), but also it's a leaky abstraction: not all UDTs are SSRIs, nor want to be SSRI-like!!

Even more, non SSRI UDTs are a second thought in this implementation: an implementer cannot even add very normal xUDT metadata (name, symbol, decimals) without subclassing the Udt class!!

Not only that, but even sub-classing SSRI is creating troubles cause the methods for getting this metadata are async, plus given the SSRI return type signature equal pure madness. We can't even properly format ErrorUdtInsufficientCoin because of this.

I'd like to propose a simple solution:

1. Completely remove the metadata methods, no way around that. Also please clean up the remaining SSRI methods return type.
2. Declare the metadata as class properties, good old public readonly ... in the constructor parameters.
3. Declare at least one static methods to instantiate UDT: all SSRI metadata is fetched at construction time.

Love & Peace, Phroi %38

[phroi]

@Hanssen0 what kind of changes would you like to make to this PR?

[Hanssen0]

@Hanssen0 what kind of changes would you like to make to this PR?

I want to merge this PR as soon as we can. It has been staying here for too long and contains modifications/updates that affect other things. I understand your concerns about SSRI types, and they are reasonable. However, given that they have already been included in previous releases (so modifying them might cause compatibility trouble) & predictably will take some time to implement, I suggest that we should discuss this further after completing other tasks & merging them.

The remaining issues are what you mentioned before: that some UDT-like cells might not store their data in the first 16 bytes. It would be nice if we could make the udtBalanceFrom method easier to override. I'm still unsure how to proceed due to my limited knowledge about what we aim to achieve with these cells. It would be great if you could propose some possible solutions.

[Hanssen0]

/canary
For testing

[github-actions]

🚀 Canary version published successfully! View workflow run

The following packages have been published to npm:

• @ckb-ccc/ccc@0.0.0-canary-20250816153008
• ckb-ccc@0.0.0-canary-20250816153008
• @ckb-ccc/connector@0.0.0-canary-20250816153008
• @ckb-ccc/connector-react@0.0.0-canary-20250816153008
• @ckb-ccc/core@0.0.0-canary-20250816153008
• @ckb-ccc/eip6963@0.0.0-canary-20250816153008
• @ckb-ccc/joy-id@0.0.0-canary-20250816153008
• @ckb-ccc/lumos-patches@0.0.0-canary-20250816153008
• @ckb-ccc/nip07@0.0.0-canary-20250816153008
• @ckb-ccc/okx@0.0.0-canary-20250816153008
• @ckb-ccc/rei@0.0.0-canary-20250816153008
• @ckb-ccc/shell@0.0.0-canary-20250816153008
• @ckb-ccc/spore@0.0.0-canary-20250816153008
• @ckb-ccc/ssri@0.0.0-canary-20250816153008
• @ckb-ccc/udt@0.0.0-canary-20250816153008
• @ckb-ccc/uni-sat@0.0.0-canary-20250816153008
• @ckb-ccc/utxo-global@0.0.0-canary-20250816153008
• @ckb-ccc/xverse@0.0.0-canary-20250816153008

[phroi]

Note for future readers: in my opinion this PR should not be merged as it stands.

This PR has the following issues:

1. There is no method for accessing the UDT value of a single input/output cell.
2. There is no way to cleanly account for cells that have UDT value, but are not UDT cells.
3. UDT is strongly tied to SSRI, but it shouldn't, as UDT is independent from SSRI.
4. UDT doesn't allow DepGroups as its CellDeps.
5. UDT Metadata behind async methods is source of unnecessary troubles.
6. completeInputs only compatible cached cells, not directly fetched from on chain.
7. Closures make the understanding of some methods more difficult for devs.

That's why I proposed another WIP PR, which addresses the aforementioned issues.

It would be great if you could propose some possible solutions.

I already proposed, I will review here 🤗

It [...] contains modifications/updates that affect other things

Yep, wouldn't it make more sense to merge those separately?

I understand your concerns about SSRI types, and they are reasonable.

The more work we put towards them, the more they are difficult to remove later on. Right?

For example, what if a non compatible SSRI-alike comes later on? And then another and another

some UDT-like cells might not store their data in the first 16 bytes. It would be nice if we could make the udtBalanceFrom method easier to override.

Yep, you may want to take a look at how infoFrom fills exactly that role in my PR

About the integration of this class in iCKB, as I was mentioning some time ago:

I gave some thoughts about this and I was thinking about keeping the iCKB UDT subclass simple and overriding just addCellDeps and getInputsInfo. (I'll know for sure only when coding tho)

It will still create some trouble as the calculateBalance result will always be lower or equal than the user real UDT worth (which includes iCKB receipts), but should be ok.

That said, currently I'm puzzled by the lack of a method to get the UDT value of a single input/output Cell.

How do you propose to address this issue @Hanssen0?

Love & Peace, Phroi %85

[Hanssen0]

Yep, wouldn't it make more sense to merge those separately?

Agree. I should divide this PR into smaller pieces so that we can merge the less controversial content.

The more work we put towards them, the more they are difficult to remove later on. Right?

Yes. But I would say this PR is not making UDT more SSRI - it's already coupled with SSRI. We can still do some optimization on top of this, as long as it doesn't make things worse.

Yep, you may want to take a look at how infoFrom fills exactly that role in my PR

Now I see more clearly about why Info existed there, not only for performance, but also for abstracting UDT infos. However, the additional thing that requires devs to maintain is still a problem. Let's see if there's a way to avoid it.

[Hanssen0]

This PR has the following issues:

1. There is no method for accessing the UDT value of a single input/output cell.
2. There is no way to cleanly account for cells that have UDT value, but are not UDT cells.
3. UDT is strongly tied to SSRI, but it shouldn't, as UDT is independent from SSRI.
4. UDT doesn't allow DepGroups as its CellDeps.
5. UDT Metadata behind async methods is source of unnecessary troubles.
6. completeInputs only compatible cached cells, not directly fetched from on chain.
7. Closures make the understanding of some methods more difficult for devs.

Most of them are not issues of this PR, but rather issues that existed in the current codebase, and should not be expected to be all solved once this PR is merged.

[Hanssen0]

Please review these three PRs first. Merging them should make this PR less messy:

#258
#259
#260

[phroi]

Good work there, I review them and they are perfect!!

Phroi %63

[Hanssen0]

Good work there, I review them and they are perfect!!

Phroi %63

Awesome! I think now we can begin to break this PR into smaller pieces. Here's my plan:

1. Let's focus on the complete* methods first. On this stage, only ordinary UDT cells will be considered. We should be able to move every UDT-related method out of Transaction after this.
2. 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.
3. Allow more than one cell deps, and also dep groups.
4. Decouple SSRI with UDT.

This PR will focus on the first stage. After we finish that, we can have new PRs for the following.

How do you think?

[phroi]

So long we are able to address, some of those issues I'm happy. Let's go with the plan!!

I reviewed the current state of UDT and it is reasonable, just surprised to see calculateInfo there!

Phroi %19

[phroi]

Looks good, in line with phase 1 expectations, feel free to merge! 👍

Let's move to phase 2!

Phroi %4

[Hanssen0]

I reviewed the current state of UDT and it is reasonable, just surprised to see calculateInfo there!

You suggested that before, and I'm happy to add that. If devs don't need to maintain them manually, it does help.

I've trimmed down a little bit of the code and removed the calculateInfo temporary (will add them back in the next PR). How do you think?

For the complete* methods, I recommend devs to use

• completeInputsByBalance
• completeInputsAll
• completeChangeToOutput
• completeChangeToLock
• completeBy (This should be most used)

for normal cases. Only use

• completeInputs
• complete

if the above methods can't satisfy what they need.

[phroi]

How do you think?

GPT-4o says:

The phrase "How do you think?" is not commonly used in English. A more natural way to ask for someone's opinion or perspective would be "What do you think?" or "How do you feel about this?"

I'm really sorry, no disrespect intended, my English has room for improvement too 🤗

I've trimmed down a little bit of the code and removed the calculateInfo temporary (will add them back in the next PR). How do you think?

Everything seems reasonable, I meant exactly that about calculateInfo

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

Phroi %17

[Hanssen0]

GPT-4o says:

The phrase "How do you think?" is not commonly used in English. A more natural way to ask for someone's opinion or perspective would be "What do you think?" or "How do you feel about this?"

I'm really sorry, no disrespect intended, my English has room for improvement too 🤗

Thank you so much for the notice! I need more practice for my English (& Grammarly helped me a lot).

I've trimmed down a little bit of the code and removed the calculateInfo temporary (will add them back in the next PR). How do you think?

Everything seems reasonable, I meant exactly that about calculateInfo

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.

[phroi]

I replied in the phase 2 of UDT PR 🤗