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

Add missing auth id(0x02~0x05) #75

Closed

Conversation

XuJiandong
Copy link
Collaborator

@XuJiandong XuJiandong commented Jan 3, 2024

NOTE

This PR is migrated to here.
The omnilock is moved to new repo.


including

  • eos
  • tron
  • bitcoin(Support UniSat and OKX wallet)
  • dogecoin

Special feature for btc/etc, now they can display meaningful messages like below:

  • btc(unisat/okx)
You're signing:
CKB (Bitcoin Layer-2) transaction: 0x{sighash_all in hex}
image
  • eth(metamask)
You're signing:
CKB transaction: 0x{sighash_all in hex}
image

Other changes:

  • Add hash functions: sha256 and ripemd160
  • Fix rust toolchains
  • Code format

including
* eos
* tron
* bitcoin
* dogecoin

Add hash functions: sha256 and ripemd160
Fix rust toolchains
Code format
@XuJiandong
Copy link
Collaborator Author

XuJiandong commented Jan 8, 2024

Review Guide:

  1. Any existing auth id branch is not modified
  2. The bitcoin auth id supports 3 routines: uncompressed, compressed and P2SH.
    Note, the uncompressed pubkey is not used in UniSat/OKX wallet. It is quite old address. The following addresses type are supported in UniSat/OKX:
  • Native SegWit (P2WPKH)
  • Nested SegWit(P2SH-P2WPKH)
  • Legacy (P2PKH)

Copy link
Member

@doitian doitian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Added some minor style suggestions.

@homura
Copy link

homura commented Jan 9, 2024

It would be helpful to put the compiled contracts in the PR description to help test or preview them more easily

update: this is compiled contracts based on 0019119
build.zip

@XuJiandong
Copy link
Collaborator Author

It would be helpful to put the compiled contracts in the PR description to help test or preview them more easily

omni_lock_commit_id_00191199.gz

unzip and check hash by ckb-cli:

❯ ckb-cli util blake2b --binary-path omni_lock_commit_id_00191199
0xdea31e1f104cf6a26ee3cbace40f6fa70e2deb556cd12fdc23eae15889d0d37b

@XuJiandong
Copy link
Collaborator Author

For those who want to test with lumos, here is the reference implementation:
https://github.com/cryptape/ckb-unisat-poc/blob/main/dapp/src/walletUnisat.ts

doitian added a commit to cryptape/ckb-dao-cobuild-poc that referenced this pull request Jan 10, 2024
What's changed:

- Switch to the deployment information of Omnilock
- args: append 00. It is the omni lock flag byte, which indicate there's
  no advanced omni lock features been used.
- `WitnessArgs.lock` size: 65 to 85, this is the size of a
`OmnilockWitnessLock` to only
  save a 65-byte signature.
- witness: instead of storing the 65-byte signature directly into
  `WitnessArgs.lock`, pack it into a `OmnilockWitnessLock`.

Omnilock Contract Error Codes For Reference:

- 8: args format is wrong
- 5: `WitnessArgs.lock` is not a valid `OmnilockWitnessLock`

[An example deposit in
testnet](https://pudge.explorer.nervos.org/transaction/0x7bc9bd9a60852bf85d7142041867fb3d92b44a0d5821835fef9b1978442e84c0)

Omnilock info:

| parameter | value |
| ----------- |
-------------------------------------------------------------------- |
| `code_hash` |
0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb |
| `hash_type` | `type` |
| `tx_hash` |
0xff234bf2fb0ad2ab5b356ceda317d3dee3efb2c55b9427ef55d9dcbf6eecbf9f |
| `index` | `0x0` |
| `dep_type` | `code` |

Related PRs:
nervosnetwork/ckb-production-scripts#75

Closes #12
@XuJiandong XuJiandong force-pushed the add-missing-auth-ids branch 2 times, most recently from 6702653 to 5e8c467 Compare January 11, 2024 03:32
* Display message on wallet with meaningful words. like below:
You're sining:

CKB transaction: 001122334455...

works for bitcoin and ethereum

* add new ethereum algorithm id(18) for displaying message
* update bitcoin algorithm id
* keep eos/tron/dogecoin as same to PW

* Add trait: ChainConfig, used for other on-chain signatures
* Add eth displaying testcase (#5)
@homura
Copy link

homura commented Jan 11, 2024

Is the new Displaying message concept in BitcoinIdentity defaulting to CKB (Bitcoin Layer-2) transaction: prefix? Has the previous hex format been deprecated?

* Add 0x at the beginning of prefix
@XuJiandong
Copy link
Collaborator Author

deployed on testnet(commit: f884d9):

parameter value
code_hash 0xf329effd1c475a2978453c8600e1eaf0bc2087ee093c3ee64cc96ec6847752cb
hash_type type
tx_hash 0xb50ef6f2e9138f4dbca7d5280e10d29c1a65e60e8a574c009a2fa4e4107e0750
index 0x0
dep_type code

XuJiandong added a commit to XuJiandong/omnilock that referenced this pull request Jan 12, 2024
migrate PR from: nervosnetwork/ckb-production-scripts#75

including
* eos
* tron
* bitcoin(Support UniSat and OKX wallet)
* dogecoin

Special feature for btc/etc, now they can display meaningful messages.

* BTC(UniSat/OKX)
You're signing:
CKB (Bitcoin Layer-2) transaction: 0x{sighash_all in hex}

* ETH(Metamask)
You're signing:
CKB transaction: 0x{sighash_all in hex}
@XuJiandong
Copy link
Collaborator Author

NOTE

This PR is migrated to here.
The omnilock is moved to new repo.

@XuJiandong XuJiandong closed this Jan 16, 2024
XuJiandong added a commit to cryptape/omnilock that referenced this pull request Jan 22, 2024
* Add missing auth id(0x02~0x05)

migrate PR from: nervosnetwork/ckb-production-scripts#75

including
* eos
* tron
* bitcoin(Support UniSat and OKX wallet)
* dogecoin

Special feature for btc/etc, now they can display meaningful messages.

* BTC(UniSat/OKX)
You're signing:
CKB (Bitcoin Layer-2) transaction: 0x{sighash_all in hex}

* ETH(Metamask)
You're signing:
CKB transaction: 0x{sighash_all in hex}

* Fix: Remove btc testcase annotation (#2)

* Change Bitcoin Prefix (#10)

---------

Co-authored-by: joii2020 <87224197+joii2020@users.noreply.github.com>
XuJiandong added a commit to XuJiandong/omnilock that referenced this pull request Jan 22, 2024
migrate PR from: nervosnetwork/ckb-production-scripts#75

including
* eos
* tron
* bitcoin(Support UniSat and OKX wallet)
* dogecoin

Special feature for btc/etc, now they can display meaningful messages.

* BTC(UniSat/OKX)
You're signing:
CKB (Bitcoin Layer-2) transaction: 0x{sighash_all in hex}

* ETH(Metamask)
You're signing:
CKB transaction: 0x{sighash_all in hex}
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

Successfully merging this pull request may close these issues.

6 participants