Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] P2SH by type Auth #4

Closed
phroi opened this issue Mar 13, 2024 · 7 comments
Closed

[Feature Request] P2SH by type Auth #4

phroi opened this issue Mar 13, 2024 · 7 comments

Comments

@phroi
Copy link

phroi commented Mar 13, 2024

Hello Cryptape, iCKB here 👋

I noticed that Cryptape is re-developing Omnilock to be CoBuild compatible and eventually Cryptape will redeploy the Omnilock. At the same time, the impeding transition to CoBuild and CoBuild OTX has forced me to re-evaluate iCKB from the contracts up.

As per Nervos Talk post, CoBuild, Omnilock and Administrator Mode, I'm evaluating a design based on Omnilock's Administrator Mode for the limit order contract. Briefly, a Limit Order design could use Omnilock as base and be unlocked either:

  • By user with signature, as usual.
  • Using Administrator Mode with Limit Order Domain Logic included as P2SH.

In this design there are two kind of cells:

Limit Order cell:
- Lock: Omnilock with Domain Logic P2SH Auth Administrator Mode 
- Type: sUDT
- Data: [amount, ... limit order data]

Domain Logic cell:
- Lock: Domain Logic to validate limit orders
- Type: ...

Considerations:

  • Including Domain Logic as Lock Script is inconvenient, but P2SH auth rules require to use lock script for validation. It would be so much more convenient to include it as Type Script, as the evolution form sUDT to xUDT showed.
  • From an OTX CoBuild perspective, the Limit Order cell would be an Action without any matching type script. Then again CoBuild validation rules require all Action objects to have a matching type script in the fully assembled CKB Transaction.
  • Limit Orders are just an example from a much broader class of DApps. If Omnilock implements P2SH by type Auth, then a new class of DApps becomes much easier to develop, even in a Cobuild OTX environment. The following cell becomes the only cell needed by both P2SH by type Auth and CoBuild OTX:
Domain Logic cell:
- Lock: ...
- Type: Domain Logic to validate limit orders

Could Cryptape implement P2SH by type Auth in Omnilock as a new Auth flag?

@XuJiandong
Copy link
Contributor

XuJiandong commented Mar 13, 2024

Could you write some pseudo code about this feature?

And a question to the design of second cell: Why is the logic in lock but not in type?

Domain Logic cell:
- Lock: Domain Logic to validate limit orders
- Type: ...

@phroi
Copy link
Author

phroi commented Mar 13, 2024

Hey @XuJiandong, thank you for your time and interest 🤗

Why is the logic in lock but not in type?

Exactly, that's the issue here! To be P2SH compatible:

When the auth flag is 0xFC, we will check against the current transaction, and there must be an input cell, whose lock script matches the auth content when hashed via blake160.

Without any modification to Omnilock, Domain Logic script would need to be present in the transaction possibly both as lock script (to satisfy P2SH requirements) and type script (to satisfy CoBuild action requirements).

While with the implementation of this feature, Domain Logic script would need to be present in the transaction only by type as this would satisfy requirements of both P2SH by type and CoBuild action.

Could you write some pseudo code about this feature?

Sure, the requested feature itself is a variation of Unlock via owner's lock script hash. Let's say 0xFB is the flag of P2SH by type, there are two scenarios:

a. Unlock via owner's type script hash in input cell:

CellDeps:
    <vec> Omnilock Script Cell
Inputs:
    <vec> Cell
        Data: <...>
        Type: <...>
        Lock:
            code_hash: Omnilock
            args: <flag: 0xFC> <lock hash: 0x1234> <Omnilock flags: 0>
    <vec> Cell
        Data: <...>
        Type: blake160 for this lock script must be 0x1234 <--- ONLY DIFFERENCE: FROM LOCK TO TYPE
        Lock: <...>
    <...>
Outputs:
    <vec> Any cell
Witnesses:
    WitnessArgs structure:
      Lock:
        signature: <MISSING>
        omni_identity: <MISSING>
        preimage: <MISSING>
      <...>

b. Unlock via owner's type script hash in output cell:

CellDeps:
    <vec> Omnilock Script Cell
    <vec> RC AdminList Cell 1
Inputs:
    <vec> Cell
        Data: <...>
        Type: <...>
        Lock:
            code_hash: Omnilock
            args: <flag: 0xFC> <lock hash: 0x1234> <Omnilock flags: 0>
    <...>
Outputs:
    <vec> Cell
        Data: <...>
        Type: blake160 for this lock script must be 0x1234
        Lock: <...>
    <...>
Witnesses:
    WitnessArgs structure:
      Lock:
        signature: <MISSING>
        omni_identity: <MISSING>
        preimage: <MISSING>
      <...>

The use in conjunction with administrator mode is a variation of Unlock via administrator's lock script hash (1). Let's say 0xFB is the flag of P2SH by type, there are two scenarios:

a. Unlock via administrator's type script hash in input cell:

CellDeps:
    <vec> Omnilock Script Cell
    <vec> RC AdminList Cell 1
Inputs:
    <vec> Cell
        Data: <...>
        Type: <...>
        Lock:
            code_hash: Omnilock
            args: <flag: 0> <pubkey hash 1> <Omnilock flags: 1> <RC AdminList Cell 1's type ID>
    <vec> Cell
        Data: <...>
        Type: blake160 for this lock script must be 0x1234 <--- ONLY DIFFERENCE: FROM LOCK TO TYPE
        Lock: <...>
    <...>
Outputs:
    <vec> Any cell
Witnesses:
    WitnessArgs structure:
      Lock:
        signature: <MISSING>
        omni_identity:
           identity: <flag: 0xFB> <lock hash: 0x1234>
           proofs: <SMT proofs for the above identity in RC AdminList Cell 1>
        preimage: <MISSING>
      <...>

b. Unlock via administrator's type script hash in output cell:

CellDeps:
    <vec> Omnilock Script Cell
    <vec> RC AdminList Cell 1
Inputs:
    <vec> Cell
        Data: <...>
        Type: <...>
        Lock:
            code_hash: Omnilock
            args: <flag: 0> <pubkey hash 1> <Omnilock flags: 1> <RC AdminList Cell 1's type ID>
    <...>
Outputs:
        <vec> Cell
        Data: <...>
        Type: blake160 for this lock script must be 0x1234
        Lock: <...>
    <...>
Witnesses:
    WitnessArgs structure:
      Lock:
        signature: <MISSING>
        omni_identity:
           identity: <flag: 0xFB> <lock hash: 0x1234>
           proofs: <SMT proofs for the above identity in RC AdminList Cell 1>
        preimage: <MISSING>
      <...>

@XuJiandong
Copy link
Contributor

XuJiandong commented Mar 14, 2024

Firstly, I think this behavior should remain untouched:

0xFC: The auth content that represents the blake160 hash of a lock script. The lock script will check if the current transaction contains an input cell with a matching lock script. Otherwise, it would return with an error. It's similar to P2SH in BTC.

We can add a new flag, like "0xFB", as below:

0xFB: The auth content that represents the blake160 hash of a type script. The script will check if the current transaction contains an input/output cell with a matching type script. Otherwise, it would return with an error.

This change is also applied to administrator mode. Please confirm this change fulfill requirement. Then I can push this propose forward.

@phroi
Copy link
Author

phroi commented Mar 14, 2024

Yes, that's 100% correct, just let me confirm the intended behavior from a CoBuild perspective:

  1. In a CoBuild non-OTX transaction, 0xFB Auth behaviour is well defined ✅
  2. In a CoBuild OTX transaction, 0xFB Auth behaviour can be interpreted into two separate ways:

a. Multiple OTX use 0xFB Auth (all same blake160 hash), but only a single cell with matching type script is required in the the fully assembled Transaction to validate all OTX. This is the intended behavior! ✅
b. Multiple OTX use 0xFB Auth (all same blake160 hash) and within each OTX a cell with matching type script exists, so multiple cells. This is not the intended behavior! ❌❌❌

Note: in a CoBuild OTX transaction the behavior between 0xFB Auth and 0xFC Auth may differ as 0xFC Auth may be 2.b, while the requested 0xFB Auth is 2.a.

@XuJiandong
Copy link
Contributor

XuJiandong commented Mar 14, 2024

Yes, In Multiple OTX, omnilock will work as 2.a, both for 0xFB(new) and 0xFC(old).

@phroi
Copy link
Author

phroi commented Mar 14, 2024

About 0xFB(new), we covered everything, so please push this proposal forward, thanks for your time!! 🙏

About 0xFC(old), please be careful with 2.a as it may lead to an attack vector. Let's assume that:

An attack can be the following:

  • Attacker will include all other cells locked with 0xFC (all same blake160 hash) in a second OTX.
  • This second OTX will be packed together with the first OTX in the same transaction.
  • These second cells will unlock thanks to 2.a.
  • Attacker will be able to gain control of these second cells.

On the other side, 0xFC(old) with 2.a is safe if matching lock script is either SighashAll or Domain Logic.

The same attack may apply to 0xFB(new), but using signature based scripts as type is so uncommon that there is no real risk.

So when documenting 0xFC (old) OTX and 0xFB (new) OTX, please remember to document this potential pitfall! 🙏

@phroi
Copy link
Author

phroi commented Mar 23, 2024

In the end @xxuejie convinced me to NOT use this possible Omnilock feature:

I do want to stress this point one more time: this albeit-working solution is also manually coded against one particular feature implemented by one particular lock, P2SH by type auth is not a commonly-defined feature implemented by all cobuild-compatible locks. We’ve learned enough lessons, I plead you and every CKB developers that are reading this to never design your app against a particular lock or type ever again.

@phroi phroi closed this as completed Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants