Magickbase / neuron-public-issues

Neuron Issues
6 stars 3 forks source link

namespaces for WalletConnect #148

Open devchenyan opened 1 year ago

devchenyan commented 1 year ago

Table of Contents

Namepaces

A namespace is a standardized object defined by the Chain Agnostic Improvement Proposal (CAIP) that ensures a common industry standard for chain agnostic purposes. You will encounter two namespaces: the proposal namespace and the session namespace when connects wallets and dapps.

Here is an example of a session approval message, passing the namespace.

{
  ...
    "namespaces": {
      "ckb": {
        "accounts": [ "ckb:mainnet:xxxxx" ],
        "chains": [ "ckb:mainnet", ... ],
        "events": [ "accountChanged", ... ],
        "methods": [ "ckb_signTransaction", ... ],
      }
    }
  ...
}

chains

In CAIP-2, ageneral blockchain identification schema is defined. This is the implementation of CAIP-2 for the CKB network.

Component Description
caip2-like chain namespace + : + chainId
namespace ckb
chainId One of (mainnet, testnet, devnet)

accounts

CAIP-10 defines a way to identify an account in any blockchain specified by CAIP-2 blockchain id.

For context, see the CAIP-10 specification.

Component Description
caip10-like account namespace + : + chainId + : + identity
namespace ckb
chainId One of (mainnet, testnet, devnet)
identity The hash obtained by the first pk of the account

methods

ckb_getAddresses

Get the active address list by specific lock script types

  • Parameters

    {
    ['xxxxxx' ] :{ // lock code hash
       page: {
         size: number
         before: number
         after: number
       },
       type: 'derived' | 'all'
    }
    }
  • Returns

    {
     'xxxxxx' : Address[],
      'xxxxxx' : Address[],
    }

ckb_generateAddresses

Allow a cold wallet to authorize dapps to generate addresses.

For context, see the doc

ckb_signTransaction

Get a transaction over the provided instructions.

ckb_signMessage

Get a signature for the provided message from the requested signer address.

events

accountChanged

Emit the event when the account changed

event: {
   name: "accountChanged",
   data: {
       account: string,
       addresses: Address[]
   },
}

addressesChanged

Emit the event when addresses changed

event: {
   name: "addressesChanged",
   data: {
        ...Address,
        changeType: 'add' | 'consume'
    }[]
}

chainChanged

Emit the event when the chain changed

event: {
   name: "chainChanged",
   data: string,
}
IronLu233 commented 1 year ago

Some information from nexus methods.

Nexus RPC Nexus events

In previous nexus design, it have these methods:

wallet_fullOwnership_getOffChainLocks

Get the off chain locks. The addresses parsed from locks are for receiving assets. image Like this in neuron.

wallet_fullOwnership_getOnChainLocks and wallet_fullOwnership_getLiveCells

Get the on chain locks. This method can make app easy to retrieve the transaction history. @zhangyouxin What the different between these two API?

wallet_fullOwnership_signData

Sign a data with a special private key. This method is for signing a message.

ckb_getBlockchainInfo

Get the blockchain info

ckb_sendTransaction

Send the transaction to blockchain. will auto inject fee and sign transaction. It is a smart method for unsigned transaction.

events: networkChanged

Trigger when user change network on the UI.


I think these methods can be implemented the methods in wallet connect. But fullOwnership prefix can be removed because we have to need to distinguish the ownership.

IronLu233 commented 1 year ago

First, I propose the chains config.

With CAIP-2. It have two parts, namespace and references. ${namespace}:${references} For convenient, the chain should be ckb. Siting this CKB docs, it have three parameter to --chain of ckb binary: mainnet, testnet and dev.

I provide two Chain types, with and without net suffix.

So the final chains is:

type Chains: 'mainnet' | 'testnet' | 'devnet'
// Or just remove all net suffix
type Chains = 'main' | 'test' | 'dev'
zhangyouxin commented 1 year ago

wallet_fullOwnership_getOnChainLocks returns used locks, and wallet_fullOwnership_getLiveCells returns their on-chain live cells.

I think we don't need to return locks or cells for DApp in neuron, just return addresses is enough, the Dapp like .bit will get cells using one of the addresses neuron has approved. And neuron can switch address with emitting address_changed event, DApp should listen to this event and do address change business.

Here is an assumpsion(maybe a fact):

inspired by poc by homura hers is a initial draft of proposal:

zhangyouxin commented 1 year ago

On Wallet connect, neuron user can select multiple addresses to add to approved list, but return only one address a time, when neuron user switch to another approved address, dapp will receive a address_changed event and do something for change address business(get balance, etc).

IronLu233 commented 1 year ago

chains: ckb:1:1 for m/44'/309'/0' with SECP256K1_BLAKE160 locks (for both external and internal addresses) ckb:1:2 for m/44'/309'/0' with OMNI locks with owner mode (considering some dapps already uses this lock) // more chains to be added

 chains: string[]; // your supported chains in CAIP-2 format e.g. ["eip155:1", "eip155:2", ...]

is these the chains in namespace? I think the chains is to distinguish which chain current wallet uses instead of different lock.

I found in EIP, there are these chain id: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md#list-of-chain-ids

CHAIN_ID Chain(s)
1 Ethereum mainnet
2 Morden (disused), Expanse mainnet
3 Ropsten
4 Rinkeby
5 Goerli
42 Kovan
1337 Geth private chains (default)

mainnet, Ropsten, Rinkeby are different network.

zhangyouxin commented 1 year ago

You are right, network infomation should also be involved in chain-id, and the reference part of chain-id should be coded to [-_a-zA-Z0-9]{1,32} to be compatible with caip-2 format

IronLu233 commented 1 year ago

Maybe we don't need address_sign_in and address_sign_out

Because there a session_delete event.

zhangyouxin commented 1 year ago

After connecting the Neuron wallet, the address given to the DApp is derived from the pubKey of the secp256k1_blake160 lock at the path m'/44'/309'/0'. However, maybe it's not very good that the wallet only support the secp256k1_blake160 lock. Otherwise, the DApp may face a lot of limitations. For example, Dotbit has deployed its own lock: https://explorer.nervos.org/transaction/0x489ec335355d028488de248f04520103ac361a61cc606b17d30f71c707405eb2.

So, how can we support other locks besides secp256k1_blake160?

Initially, I thought of including lock information in the chainId, such as ckb:mainnet-bip44-secp256k1_blake160. However, the problem with this approach is that it becomes difficult to add new locks. Is it possible that the "address" returned after connecting the Neuron wallet is actually a pubKey? If that's the case, DApp developers can use the pubKey to generate an address with the lock they prefer to use.

pros:

cons:

yanguoyu commented 1 year ago

support any lock

It seems meaningless to support any lock, because for an unknown lock, Neuron maybe not know how to sign and can't show the assets, then what is the meaning of its existence? To get information from a local node?

zhangyouxin commented 1 year ago

support any lock

It seems meaningless to support any lock, because for an unknown lock, Neuron maybe not know how to sign and can't show the assets, then what is the meaning of its existence? To get information from a local node?

I've thought about this question, in signing transacion, the DApp should tell wallet which lock to sign by passing the codeHash to wallet before really calling sign transaction, to achive that, wallet needs to provide some interfaces:

the wallet_signTransaction will sign the inputs of tx which has the lock with codeHash in approved list, secp256k1 codeHash is by default approved by all sites.

delare const myCodeHash: string;
const approved = await neuron.request({ method: "wallet_isContractApproved", params: [myCodeHash] });
if (!approved) {
  await neuron.request({ method: "wallet_approveContract", params: [myCodeHash] });
}
await neuron.request({ method: "wallet_signTransaction", params: { tx } });
yanguoyu commented 1 year ago

I've thought about this question, in signing transacion, the DApp should tell wallet which lock to sign by passing the codeHash to wallet before really calling sign transaction, to achive that, wallet needs to provide some interfaces:

  • wallet_approveContract(codeHash: Hash)
  • wallet_isContractApproved(codeHash: Hash)
  • wallet_signTransaction(tx: Transaction)

the wallet_signTransaction will sign the inputs of tx which has the lock with codeHash in approved list, secp256k1 codeHash is by default approved by all sites.

delare const myCodeHash: string;
const approved = await neuron.request({ method: "wallet_isContractApproved", params: [myCodeHash] });
if (!approved) {
  await neuron.request({ method: "wallet_approveContract", params: [myCodeHash] });
}
await neuron.request({ method: "wallet_signTransaction", params: { tx } });

Maybe we can throw an exception when calling signTransaction with an unknown lock or type. Because developers should know which codeHash is supported in Neuron. And if they want us to support other codeHash, they can propose a requirement. Or we can give a parameter to support skip signing for unknown lock or type, I'm not sure if this is reasonable.

Keith-CY commented 1 year ago

FYI, a historical RFC draft: https://talk.nervos.org/t/rfc-wallet-authorization-spec-proposal/4962

Keith-CY commented 1 year ago

FYI how keypering handled arbitrary scripts: https://nervosnetwork.github.io/keypering/#/manual?id=lock-plug-ins

IronLu233 commented 1 year ago

FYI how keypering handled arbitrary scripts: https://nervosnetwork.github.io/keypering/#/manual?id=lock-plug-ins

sign(context: SignContext, rawTx: RawTransaction, config: Config): Promise<RawTransaction>;
export enum SignatureAlgorithm {
  secp256k1 = "secp256k1",
  secp256r1 = "secp256r1",
  schnorr = "schnorr",
}

Seems the custom lock should provide a method for mapping unsigned transaction to a witness. (The generated witness are in it's output transaction) And the sign algorithm is limited on several presets, for example, sign the message with secp256k1 algorithm and private key.

Keith-CY commented 1 year ago

FYI how keypering handled arbitrary scripts: nervosnetwork.github.io/keypering/#/manual?id=lock-plug-ins

sign(context: SignContext, rawTx: RawTransaction, config: Config): Promise<RawTransaction>;
export enum SignatureAlgorithm {
  secp256k1 = "secp256k1",
  secp256r1 = "secp256r1",
  schnorr = "schnorr",
}

Seems the custom lock should provide a method for mapping unsigned transaction to a witness. (The generated witness are in it's output transaction) And the sign algorithm is limited on several presets, for example, sign the message with secp256k1 algorithm and private key.

Correct.

The plugged-in lock script should inform wallet how to sign a transaction, or to say, the algorithm is injected into the wallet. But the curve to sign a message is limited to the built-in preset.

homura commented 1 year ago

I made a simple proposal, but posting it here may slow down the issue rendering, so I post it on the HackMD for now, and I'll move it here if you think it is necessary to migrate it over for discussion

Keith-CY commented 1 year ago

FYI, design from CKBull https://gravel-secure-b5c.notion.site/Mobile-signing-feature-design-32d04174dbde47cf8d5109659b9f1a15

Keith-CY commented 1 year ago

accountAddress is used in account section, it's confusing because account and address are different concepts at different levels. I would suggest renaming accountAddress to identity

Keith-CY commented 1 year ago

Final draft is available at https://github.com/Magickbase/neuron-public-issues/blob/b0b9ddacc8daf9286da1d44d83fbfca73c9498a1/Neuron-PRDs/WalletConnect/GeneralComponent.md and https://github.com/Magickbase/neuron-public-issues/blob/b0b9ddacc8daf9286da1d44d83fbfca73c9498a1/Neuron-PRDs/WalletConnect/FAQ_cn.md

Keith-CY commented 10 months ago

Hold on to the implementation based on wallet connect because there's a new protocol for these scenarios