-
Notifications
You must be signed in to change notification settings - Fork 39
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: add ECDSA signature support #147
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
{ | ||
"typescript.tsdk": "node_modules/typescript/lib" | ||
"typescript.tsdk": "node_modules/typescript/lib", | ||
"editor.formatOnSave": false | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
build/ | ||
dist/ | ||
coverage/ | ||
yarn-error.log |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
{ | ||
"name": "btcsnap", | ||
"version": "2.0.3", | ||
"version": "2.0.4", | ||
"description": "btcsnap: Metamask snap to manage your bitcoin", | ||
"author": "aaronisme <[email protected]>", | ||
"homepage": "", | ||
|
@@ -67,6 +67,7 @@ | |
"@types/bs58check": "^2.1.0", | ||
"@types/create-hash": "^1.2.2", | ||
"@types/crypto-js": "^4.1.1", | ||
"@types/secp256k1": "4.0.6", | ||
"prettier": "^2.7.1", | ||
"through2": "^4.0.2" | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
export const SignMessageErrors = { | ||
DerivationPathNotSupported: { | ||
code: 40000, | ||
message: 'Derivation path not supported' | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import {SnapMock} from '../__mocks__/snap'; | ||
import {signMessage} from '../signMessage'; | ||
import * as bitcoinMessage from 'bitcoinjs-message'; | ||
import {bip44} from './fixtures/bitcoinNode'; | ||
|
||
describe('signMessage', () => { | ||
const snapStub = new SnapMock(); | ||
const domain = 'www.justsnap.io'; | ||
const message = 'test message'; | ||
const derivationPath = ['m', "49'", "0'", "0'", '0', '0'].join('/'); | ||
|
||
beforeEach(() => { | ||
snapStub.rpcStubs.snap_manageState.mockResolvedValue({network: 'test'}); | ||
}); | ||
|
||
afterEach(() => { | ||
snapStub.reset(); | ||
}); | ||
|
||
it('should sign message using ECDSA', async () => { | ||
snapStub.rpcStubs.snap_dialog.mockResolvedValue(true); | ||
snapStub.rpcStubs.snap_getBip32Entropy.mockResolvedValue(bip44.slip10Node); | ||
|
||
const result = await signMessage(domain, snapStub, message, derivationPath); | ||
|
||
expect( | ||
bitcoinMessage.verify(message, result.address, result.signature), | ||
).toBeTruthy(); | ||
}); | ||
|
||
it('should reject the sign request and throw error if user reject the sign message request', async () => { | ||
snapStub.rpcStubs.snap_dialog.mockResolvedValue(false); | ||
|
||
await expect( | ||
signMessage(domain, snapStub, message, derivationPath, 'ecdsa'), | ||
).rejects.toThrowError('User reject the sign request'); | ||
}); | ||
|
||
|
||
it('should throw error if protocol is not supported', async () => { | ||
await expect( | ||
signMessage(domain, snapStub, message, derivationPath, 'bip322'), | ||
).rejects.toThrowError('Action not supported'); | ||
}); | ||
|
||
it("should return valid address if network is mainnet", async () => { | ||
snapStub.rpcStubs.snap_dialog.mockResolvedValue(true); | ||
snapStub.rpcStubs.snap_getBip32Entropy.mockResolvedValue(bip44.slip10Node); | ||
snapStub.rpcStubs.snap_manageState.mockResolvedValue({network: 'main'}); | ||
|
||
const result = await signMessage(domain, snapStub, message, derivationPath); | ||
|
||
expect(result.address.startsWith('3')).toBeTruthy(); | ||
}); | ||
|
||
it("should return valid address if network is testnet", async () => { | ||
snapStub.rpcStubs.snap_dialog.mockResolvedValue(true); | ||
snapStub.rpcStubs.snap_getBip32Entropy.mockResolvedValue(bip44.slip10Node); | ||
snapStub.rpcStubs.snap_manageState.mockResolvedValue({network: 'test'}); | ||
|
||
const result = await signMessage(domain, snapStub, message, derivationPath); | ||
|
||
expect(result.address.startsWith('2')).toBeTruthy(); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
import bitcoinMessage from 'bitcoinjs-message'; | ||
import {SnapError, RequestErrors} from '../errors'; | ||
import {BitcoinNetwork, ScriptType, Snap} from '../interface'; | ||
import {getPersistedData} from '../utils'; | ||
import {heading, panel, text, divider} from '@metamask/snaps-ui'; | ||
import {getNetwork} from '../bitcoin/getNetwork'; | ||
import * as bitcoin from 'bitcoinjs-lib'; | ||
import { getHDNode } from '../utils/getHDNode'; | ||
import { getScriptType } from '../utils/getScriptType'; | ||
import { SignMessageErrors } from '../errors/constant/SignMessageErrors'; | ||
|
||
|
||
export const signMessage = async ( | ||
domain: string, | ||
snap: Snap, | ||
message: string, | ||
derivationPath: string, | ||
// TODO: implement bip322 message signing | ||
protocol: 'ecdsa' | 'bip322' = 'ecdsa', | ||
) => { | ||
if (protocol !== 'ecdsa') { | ||
throw SnapError.of(RequestErrors.ActionNotSupport); | ||
} | ||
|
||
const snapNetwork = await getPersistedData<BitcoinNetwork>( | ||
snap, | ||
'network', | ||
'' as BitcoinNetwork, | ||
); | ||
|
||
const scriptType = getScriptType(derivationPath); | ||
|
||
if (!scriptType || ![ScriptType.P2PKH, ScriptType.P2SH_P2WPKH].includes(scriptType)) { | ||
throw SnapError.of(SignMessageErrors.DerivationPathNotSupported); | ||
} | ||
|
||
const path = derivationPath.split('/'); | ||
const btcNetwork = getNetwork(snapNetwork); | ||
|
||
if (snapNetwork !== BitcoinNetwork.Main) { | ||
path[2] = "1'"; | ||
} | ||
|
||
const {publicKey, privateKey} = await getHDNode(snap, path.join('/')); | ||
|
||
const address = bitcoin.payments.p2sh({ | ||
redeem: bitcoin.payments.p2wpkh({ | ||
pubkey: publicKey, | ||
network: btcNetwork, | ||
}), | ||
network: btcNetwork, | ||
}).address as string; | ||
|
||
const result = await snap.request({ | ||
method: 'snap_dialog', | ||
params: { | ||
type: 'confirmation', | ||
content: panel([ | ||
heading('Signature Request'), | ||
text(`Please verify this sign message request from ${domain}`), | ||
divider(), | ||
text("Only confirm this message if you approve the content and trust the requesting site."), | ||
panel([ | ||
text(`**Address**:\n ${address}`), | ||
text(`**Network**:\n ${snapNetwork === BitcoinNetwork.Main ? 'Mainnet' : 'Testnet'}`), | ||
text(`**Message**:\n ${message}`), | ||
]), | ||
divider(), | ||
text(`By signing this message, you verify that you own the account without broadcasting any on-chain transactions. This signature does not allow transactions to be broadcast on your behalf. Only sign messages that you trust.`) | ||
]), | ||
}, | ||
}); | ||
|
||
if (!result) { | ||
throw SnapError.of(RequestErrors.RejectSign); | ||
} | ||
|
||
const signature = bitcoinMessage.sign(message, privateKey, true, { | ||
segwitType: scriptType === ScriptType.P2WPKH ? "p2wpkh" : "p2sh(p2wpkh)", | ||
}); | ||
|
||
return { | ||
signature: signature.toString('base64'), | ||
address, | ||
}; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { ScriptType } from "../../interface"; | ||
import { getScriptType } from "../getScriptType"; | ||
|
||
describe('isDerivationPathSupported', () => { | ||
it('should return ScriptType for supported derivation paths', () => { | ||
expect(getScriptType("m/44'/0'/0'/0/0")).toBe(ScriptType.P2PKH); | ||
expect(getScriptType("m/49'/0'/0'/0/0")).toBe(ScriptType.P2SH_P2WPKH); | ||
expect(getScriptType("m/84'/0'/0'/0/0")).toBe(ScriptType.P2WPKH); | ||
}); | ||
|
||
it('should return null for unsupported derivation paths', () => { | ||
expect(getScriptType("m/86'/0'/0'/0/0")).toBe(null); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import {ScriptType} from 'interface'; | ||
import {pathMap} from '../rpc/getExtendedPublicKey'; | ||
|
||
export const getScriptType = (derivationPath: string) => { | ||
const path = derivationPath.split('/'); | ||
for (const scriptType in pathMap) { | ||
if (pathMap[scriptType as ScriptType].every((v, i) => v === path[i])) { | ||
return scriptType as ScriptType; | ||
} | ||
} | ||
|
||
return null; | ||
}; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A warning should be included, if the message is not an hummable readable string, like a hex string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've included warning for any message, the same way metamask does.