-
Notifications
You must be signed in to change notification settings - Fork 19
Conversation
@mikelord007 is attempting to deploy a commit to the Zeta Team on Vercel. A member of the Team first needs to authorize it. |
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.
@mikelord007 please, fix TypeScript errors to make sure it builds with yarn build
:
./app/btcintegration/page.tsx:53:28
Type error: Parameter 'params' implicitly has an 'any' type.
51 | }
52 |
> 53 | const callXDefi = async (params) => {
| ^
54 | if (!window.xfi) return alert("XDEFI wallet not installed")
55 | const wallet = window.xfi
56 | window.xfi.bitcoin.changeNetwork("testnet")
error Command failed with exit code 1.
Hm, the issue with Xverse is that it's setting OP_RETURN as the first output. ZetaChain expects OP_RETURN to be the second output. https://blockstream.info/testnet/tx/c367cd678467d911c3b7c1b48c95aefaabd427636c655e3e80421b882ef33c5e |
@lukema95 please, review. Once this is merged, please, use this as a jumping-off point to add OKX support. Thanks! |
hey, have pushed fix for build errors and made OP_RETURN second output! |
I think it looks good, no further suggestions from me. Might need some time to get BTC to test again. |
Related: secretkeylabs/sats-connect#143 |
@mikelord007 one last thing we need to take care of is formatting the contents of OP_RETURN in binary rather than UTF-8. Here's an example of a successful transaction from Bitcoin (not using Xverse): https://mempool.space/testnet/tx/995cdc393ac10ab90a53ca12abf291feabc94f9ba2c68fe9259f30498c2d4ff9 Notice, This, on the other hand, is incorrectly formatted (this is how the code in this PR works right now): https://mempool.space/testnet/tx/d5ae072afa68df5105f4307bd9567bca37d11052661d959c3f2b7ac19e77f390 |
@mikelord007 @fadeev After I changed the code to look like this, I can sign multiple UTXOs normally:
|
WalkthroughWalkthroughThe recent updates introduce a comprehensive Bitcoin transaction integration feature within an application. A new component, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BtcIntegration
participant WalletAPI
participant XverseUtils
User->>BtcIntegration: Enter transaction details
BtcIntegration->>BtcIntegration: Validate inputs
BtcIntegration->>WalletAPI: Select wallet & request connection
WalletAPI-->>BtcIntegration: Connection status
BtcIntegration->>XverseUtils: Create transaction
XverseUtils-->>BtcIntegration: Return PSBT
BtcIntegration->>WalletAPI: Sign PSBT
WalletAPI-->>BtcIntegration: Signing status
BtcIntegration-->>User: Show transaction outcome
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (5)
app/btcintegration/xverse-utils.ts (2)
14-31
: Ensure Comprehensive Error HandlingThe function
fetchUtxo
handles errors well, but consider adding more specific error messages for better debugging.- throw new Error("Failed to fetch UTXO") + throw new Error(`Failed to fetch UTXO for address ${address}: ${response.statusText}`)
93-131
: Use Consistent Error HandlingEnsure consistent error handling by providing more specific error messages and logging.
- alert("Error while signing") + alert(`Error while signing: ${err.message}`)app/btcintegration/page.tsx (3)
66-94
: Improve Error Handling in XDEFI Wallet InteractionConsider providing more specific error messages for better debugging and user feedback.
- return alert(`Couldn't send transaction, ${JSON.stringify(err)}`) + return alert(`Couldn't send transaction: ${err.message}`)
96-107
: Improve Error Handling in UniSat Wallet InteractionConsider providing more specific error messages for better debugging and user feedback.
- return alert(`Couldn't send transaction, ${JSON.stringify(e)}`) + return alert(`Couldn't send transaction: ${e.message}`)
110-127
: Improve Error Handling in Xverse Wallet InteractionConsider providing more specific error messages for better debugging and user feedback.
- alert("wallet connection failed") + alert("Wallet connection failed. Please try again.")
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
package-lock.json
is excluded by!**/package-lock.json
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (3)
- app/btcintegration/page.tsx (1 hunks)
- app/btcintegration/xverse-utils.ts (1 hunks)
- package.json (2 hunks)
Additional context used
Biome
app/btcintegration/page.tsx
[error] 13-13: Shouldn't redeclare 'Wallet'. Consider to delete it or rename it.
'Wallet' is defined here:
(lint/suspicious/noRedeclare)
[error] 43-43: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
Additional comments not posted (5)
package.json (3)
37-37
: Dependency Addition: @scure/baseThe
@scure/base
package has been added as a dependency. Ensure that this package is necessary and correctly used in the codebase.
56-56
: Dependency Addition: micro-btc-signerThe
micro-btc-signer
package has been added as a dependency. Ensure that this package is necessary and correctly used in the codebase.
63-63
: Dependency Addition: sats-connectThe
sats-connect
package has been added as a dependency. Ensure that this package is necessary and correctly used in the codebase.app/btcintegration/xverse-utils.ts (2)
78-83
: Optimize OP_RETURN Output HandlingThe OP_RETURN output is correctly placed, but consider ensuring that the memo content is properly formatted as binary instead of UTF-8.
- const opReturn = btc.Script.encode(["RETURN", Buffer.from(memo, "utf8")]) + const opReturn = btc.Script.encode(["RETURN", Buffer.from(memo, "binary")])
133-133
: Export StatementThe export statement is correct and aligns with the functions defined in the file.
async function createTransaction( | ||
publickkey: string, | ||
senderAddress: string, | ||
params: Params | ||
): Promise<{ psbtB64: string; utxoCnt: number }> { | ||
const publicKey = hex.decode(publickkey) | ||
|
||
const p2wpkh = btc.p2wpkh(publicKey, bitcoinTestnet) | ||
const p2sh = btc.p2sh(p2wpkh, bitcoinTestnet) | ||
|
||
const recipientAddress = "tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur" | ||
if (!senderAddress) { | ||
throw new Error("Error: no sender address") | ||
} | ||
if (!recipientAddress) { | ||
throw new Error("Error: no recipient address in ENV") | ||
} | ||
|
||
const output = await fetchUtxo(senderAddress) | ||
|
||
const tx = new btc.Transaction({ | ||
allowUnknowOutput: true, | ||
}) | ||
|
||
let utxoCnt = 0 | ||
|
||
output.forEach((utxo) => { | ||
tx.addInput({ | ||
txid: utxo.txid, | ||
index: utxo.vout, | ||
witnessUtxo: { | ||
script: p2sh.script, | ||
amount: BigInt(utxo.value), | ||
}, | ||
witnessScript: p2sh.witnessScript, | ||
redeemScript: p2sh.redeemScript, | ||
}) | ||
utxoCnt += 1 | ||
}) | ||
|
||
const changeAddress = senderAddress | ||
|
||
const memo = `${params.contract}${params.message}`.toLowerCase() | ||
|
||
const opReturn = btc.Script.encode(["RETURN", Buffer.from(memo, "utf8")]) | ||
tx.addOutputAddress(recipientAddress, BigInt(params.amount), bitcoinTestnet) | ||
tx.addOutput({ | ||
script: opReturn, | ||
amount: BigInt(0), | ||
}) | ||
tx.addOutputAddress(changeAddress, BigInt(800), bitcoinTestnet) | ||
|
||
const psbt = tx.toPSBT(0) | ||
|
||
const psbtB64 = base64.encode(psbt) | ||
|
||
return { psbtB64, utxoCnt } | ||
} |
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.
Typographical Error in Parameter Name
The parameter publickkey
should be corrected to publicKey
.
- publickkey: string,
+ publicKey: string,
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
async function createTransaction( | |
publickkey: string, | |
senderAddress: string, | |
params: Params | |
): Promise<{ psbtB64: string; utxoCnt: number }> { | |
const publicKey = hex.decode(publickkey) | |
const p2wpkh = btc.p2wpkh(publicKey, bitcoinTestnet) | |
const p2sh = btc.p2sh(p2wpkh, bitcoinTestnet) | |
const recipientAddress = "tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur" | |
if (!senderAddress) { | |
throw new Error("Error: no sender address") | |
} | |
if (!recipientAddress) { | |
throw new Error("Error: no recipient address in ENV") | |
} | |
const output = await fetchUtxo(senderAddress) | |
const tx = new btc.Transaction({ | |
allowUnknowOutput: true, | |
}) | |
let utxoCnt = 0 | |
output.forEach((utxo) => { | |
tx.addInput({ | |
txid: utxo.txid, | |
index: utxo.vout, | |
witnessUtxo: { | |
script: p2sh.script, | |
amount: BigInt(utxo.value), | |
}, | |
witnessScript: p2sh.witnessScript, | |
redeemScript: p2sh.redeemScript, | |
}) | |
utxoCnt += 1 | |
}) | |
const changeAddress = senderAddress | |
const memo = `${params.contract}${params.message}`.toLowerCase() | |
const opReturn = btc.Script.encode(["RETURN", Buffer.from(memo, "utf8")]) | |
tx.addOutputAddress(recipientAddress, BigInt(params.amount), bitcoinTestnet) | |
tx.addOutput({ | |
script: opReturn, | |
amount: BigInt(0), | |
}) | |
tx.addOutputAddress(changeAddress, BigInt(800), bitcoinTestnet) | |
const psbt = tx.toPSBT(0) | |
const psbtB64 = base64.encode(psbt) | |
return { psbtB64, utxoCnt } | |
} | |
async function createTransaction( | |
publicKey: string, | |
senderAddress: string, | |
params: Params | |
): Promise<{ psbtB64: string; utxoCnt: number }> { | |
const publicKey = hex.decode(publicKey) | |
const p2wpkh = btc.p2wpkh(publicKey, bitcoinTestnet) | |
const p2sh = btc.p2sh(p2wpkh, bitcoinTestnet) | |
const recipientAddress = "tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur" | |
if (!senderAddress) { | |
throw new Error("Error: no sender address") | |
} | |
if (!recipientAddress) { | |
throw new Error("Error: no recipient address in ENV") | |
} | |
const output = await fetchUtxo(senderAddress) | |
const tx = new btc.Transaction({ | |
allowUnknowOutput: true, | |
}) | |
let utxoCnt = 0 | |
output.forEach((utxo) => { | |
tx.addInput({ | |
txid: utxo.txid, | |
index: utxo.vout, | |
witnessUtxo: { | |
script: p2sh.script, | |
amount: BigInt(utxo.value), | |
}, | |
witnessScript: p2sh.witnessScript, | |
redeemScript: p2sh.redeemScript, | |
}) | |
utxoCnt += 1 | |
}) | |
const changeAddress = senderAddress | |
const memo = `${params.contract}${params.message}`.toLowerCase() | |
const opReturn = btc.Script.encode(["RETURN", Buffer.from(memo, "utf8")]) | |
tx.addOutputAddress(recipientAddress, BigInt(params.amount), bitcoinTestnet) | |
tx.addOutput({ | |
script: opReturn, | |
amount: BigInt(0), | |
}) | |
tx.addOutputAddress(changeAddress, BigInt(800), bitcoinTestnet) | |
const psbt = tx.toPSBT(0) | |
const psbtB64 = base64.encode(psbt) | |
return { psbtB64, utxoCnt } | |
} |
const tss = "tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur" | ||
if (contractAddress.length !== 42) | ||
return alert("Not a valid contract address") | ||
if (isUndefined(amount) || isNaN(amount)) |
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.
Use Number.isNaN for Type Safety
Replace isNaN
with Number.isNaN
to avoid type coercion issues.
- if (isUndefined(amount) || isNaN(amount))
+ if (isUndefined(amount) || Number.isNaN(amount))
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (isUndefined(amount) || isNaN(amount)) | |
if (isUndefined(amount) || Number.isNaN(amount)) |
Tools
Biome
[error] 43-43: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
const BtcIntegration = () => { | ||
const [contractAddress, setContractAddress] = useState("") | ||
const [message, setMessage] = useState("") | ||
const [amount, setAmount] = useState<number | undefined>() | ||
const [selectedWallet, setSelectedWallet] = useState<Wallet>("XDefi") | ||
|
||
const sendTransaction = async () => { | ||
const tss = "tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur" | ||
if (contractAddress.length !== 42) | ||
return alert("Not a valid contract address") | ||
if (isUndefined(amount) || isNaN(amount)) | ||
return alert("Amount must be a number") | ||
|
||
const params = { | ||
contract: contractAddress.slice(2), | ||
message: message.slice(2), | ||
amount, | ||
tss, | ||
} | ||
|
||
switch (selectedWallet) { | ||
case "XDefi": | ||
await callXDefi(params) | ||
break | ||
case "UniSat": | ||
await callUniSat(params) | ||
break | ||
case "XVerse": | ||
await callXverse(params) | ||
break | ||
} |
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.
Avoid Redeclaring 'Wallet'
The Wallet
type is redeclared. Consider renaming it to avoid conflicts.
- type Wallet = "XDefi" | "UniSat" | "XVerse"
+ type WalletType = "XDefi" | "UniSat" | "XVerse"
Committable suggestion was skipped due to low confidence.
Tools
Biome
[error] 43-43: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
@mikelord007 thanks for working on this! As the example frontend repo is getting deprecated, I've migrated the changes proposed in this PR into the UniversalKit repo: zeta-chain/universalkit#8 |
Summary by CodeRabbit
New Features
Bug Fixes
Chores