cryptape / omnilock

2 stars 4 forks source link

[Feature Request] P2SH by type Auth #4

Closed phroi closed 5 months ago

phroi commented 6 months ago

Hello Cryptape, iCKB here :wave:

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:

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:

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 commented 6 months ago

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 commented 6 months ago

Hey @XuJiandong, thank you for your time and interest :hugs:

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

phroi commented 6 months ago

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

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:

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! :pray:

phroi commented 5 months ago

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.