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

feat: supported bitcoin auth in omnilock #607

Merged
merged 17 commits into from
Jan 18, 2024
Merged

Conversation

homura
Copy link
Collaborator

@homura homura commented Jan 11, 2024

Description

Fixes #604

This PR supported Bitcoin auth in Omnilock

import { bitcoin, createOmnilock } from '@ckb-lumos/lumos/common-scripts'

// create the corresponding Omnilock-Bitcoin lock
const lock = createOmnilock({
  auth: { flag:'BITCOIN', address: (await window.unisat.requestAccounts)[0] }
})

// sign the transaction
const message = txSkeleton.get('signingEntries').get(n).message
const signature = bitcoin.signMessage(message, 'ecdsa', window.unisat)
const sealedTx = sealTransaction(txSkeleton, [signature])

Example

git checkout origin/omnilock-bitcoin-auth
pnpm install
pnpm build
cd examples/omni-lock-unisat
pnpm install
npm start

ckb-via-unisat

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • unit test
  • example

Copy link

vercel bot commented Jan 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
lumos-website ✅ Ready (Inspect) Visit Preview Jan 18, 2024 7:25am

@homura homura marked this pull request as draft January 11, 2024 04:27
Copy link

codecov bot commented Jan 11, 2024

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (4f29e47) 87.04% compared to head (0467766) 87.29%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #607      +/-   ##
===========================================
+ Coverage    87.04%   87.29%   +0.25%     
===========================================
  Files          117      118       +1     
  Lines        23782    23983     +201     
  Branches      2416     2459      +43     
===========================================
+ Hits         20701    20936     +235     
+ Misses        3039     3005      -34     
  Partials        42       42              
Files Coverage Δ
packages/debugger/src/constants.ts 100.00% <100.00%> (ø)
packages/debugger/src/context.ts 92.43% <100.00%> (ø)
packages/debugger/src/executor.ts 98.85% <100.00%> (+0.14%) ⬆️
packages/common-scripts/src/omnilock.ts 91.14% <92.85%> (+1.44%) ⬆️
packages/common-scripts/src/omnilock-bitcoin.ts 85.18% <85.18%> (ø)

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f29e47...0467766. Read the comment docs.

@homura homura marked this pull request as ready for review January 11, 2024 07:56
export function decodeAddress(address: string): ArrayLike<number> {
try {
// Bech32
if (address.startsWith("bc1")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

both Native Segwit and Taproot addresses start with bc1 here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the taproot address is encoded via bech32m so it will fail when trying to decode via bech32, the following catch block will throw an error to tell the user that the p2tr is not supported yet

@homura homura marked this pull request as draft January 11, 2024 09:10
@homura
Copy link
Collaborator Author

homura commented Jan 11, 2024

Hold on, a new concept "display message" comes to Omnilock that prepends a human-readable prefix before the transaction digest, like CKB (Bitcoin Layer-2) transaction: <digest>. So we should update the signing part in this PR

@homura homura changed the title feat: supported bitcoin auth in omnilock [HOLD ON]feat: supported bitcoin auth in omnilock Jan 12, 2024
@homura homura changed the title [HOLD ON]feat: supported bitcoin auth in omnilock feat: supported bitcoin auth in omnilock[HOLD ON] Jan 12, 2024
# Conflicts:
#	packages/common-scripts/package.json
@homura homura changed the title feat: supported bitcoin auth in omnilock[HOLD ON] feat: supported bitcoin auth in omnilock Jan 15, 2024
@homura homura marked this pull request as ready for review January 17, 2024 06:16
examples/omni-lock-unisat/README.md Outdated Show resolved Hide resolved
examples/omni-lock-unisat/README.md Outdated Show resolved Hide resolved
packages/common-scripts/src/omnilock.ts Outdated Show resolved Hide resolved
packages/common-scripts/src/omnilock.ts Outdated Show resolved Hide resolved
packages/common-scripts/src/omnilock-bitcoin.ts Outdated Show resolved Hide resolved
@homura homura merged commit b44fb39 into develop Jan 18, 2024
9 checks passed
@homura homura deleted the omnilock-bitcoin-auth branch January 18, 2024 07:44
@github-actions github-actions bot mentioned this pull request Jan 18, 2024
@homura homura modified the milestone: 0.22.0 Mar 5, 2024
@github-actions github-actions bot mentioned this pull request Mar 12, 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

Successfully merging this pull request may close these issues.

Support new auth flag bitcoin via unisat in Omnilock
3 participants