-
Notifications
You must be signed in to change notification settings - Fork 42
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
Enable taproot address #408
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Some test cases would be good please :)
Tests |
73a3f39
to
f4a8be9
Compare
bc51614
to
ccab344
Compare
2c98bcc
to
4a356f2
Compare
4a356f2
to
ffab55c
Compare
ffab55c
to
bff7f3c
Compare
bff7f3c
to
7261ca5
Compare
c77a098
to
5247fb9
Compare
vi.mock(import('@scure/btc-signer'), async (importOriginal) => { | ||
const actual = await importOriginal(); | ||
|
||
return { | ||
...actual, | ||
selectUTXO: vi.fn(actual.selectUTXO), | ||
}; | ||
}); |
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.
What is the purpose of this?
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.
it allows us to check what arguments it was called with.
- vi.fn - mock/spy function
- actual.selectUTXO - original function implementation
so when it will be called it will run the same code + we can spy on arguments/errors/return values
sdk/test/utxo.test.ts
Outdated
const maxSpendableBalance = cardinalOutputs.reduce((acc, output) => acc + output.value, 0); | ||
|
||
// spend 90% of max spendable amount | ||
await createBitcoinPsbt(paymentAddress, paymentAddress, Math.floor(maxSpendableBalance * 0.9), pubkey); |
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 think we should check this fails, check the balance of the address including ordinals, try to create the UTXO and check it throws an error that the account has insufficient balance. I think this test is not really doing anything useful right now since we're checking only the cardinal balance.
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.
This test is checking that when we create psbt none of the possible inputs contains inscriptions
-- @sander2 wanted that to be tested.
I've added a test that checks if it throws an error if we try to spend max balance including ordinals. I think this new test is more general and works (throws) with all types of addresses, not just p2tr.
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.
Hmm actually yeah you are right that this test is still valid
f06e1cf
to
4ba6d93
Compare
No description provided.