RGB++ core#368
Conversation
* Simplify MapLru, while improving Complexity Closes ckb-devrel#243 * Apply suggestions from code %2 Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Allow nullable Values * Switch from non-null assertion to type assertion * Create six-steaks-grab.md --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Hanssen <hanssen0@hanssen0.com>
* Improve performance of Script & OutPoint eq %143 * Fix typedoc copy-paste mishap Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Dedupe strings in an interleaved way %27 * Revert manual interning --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
It's definitely a mistake to name `CellOnChain` `Cell`, but there is nothing we can do with that right now. To avoid more duplicate code, `CellAny` was added to represent a cell that's on-chain or off-chain.
This reverts commit 4393803. It will be moved to a new branch `feat/udt`
This reverts commit 507c6e8. This information is outdated
* feat(core): `hexFrom` passthru normalized hex and `numToHex` enforce hex normalization * feat(core): improve `isHex` * feat(core): improve `isHex` parameter
… add cellDeps array null checks
…lock args parsing, and update `btcTxid` type.
…gtest` address type detection, update JSDoc comments
…ctions directly and fix BigInt exponentiation for token decimals.
…es and leverage ccc utilities for hashing and byte conversions.
…ons and UDT issuance, and remove Lumos dependencies
…PI client into dedicated classes
…rsing compatibility and remove Buffer dependencies
🦋 Changeset detectedLatest commit: dc72c80 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for appccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for docsccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for liveccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for apiccc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the @ckb-ccc/rgbpp package, providing RGB++ protocol support including Bitcoin account management, transaction construction, and SPV proof integration. It also enhances the core package with reduce and reduceAsync utilities for iterables, hex normalization optimizations, and updated script configurations. Review feedback recommends several refinements for the new package: parallelizing UTXO processing to improve performance, removing redundant nullish coalescing in fee calculations, replacing console.log with a configurable logger, optimizing redundant byte conversions in commitment logic, and calling .unref() on timers to ensure the library doesn't block process exits.
| const inputs: TxInputData[] = []; | ||
| // TODO: parallel | ||
| for (const utxoSeal of uniqueSeals) { | ||
| const tx = await this.dataSource.getTransaction(utxoSeal.txid); | ||
| if (!tx) { | ||
| throw new ErrorBtcTransactionNotFound(utxoSeal.txid); | ||
| } | ||
| const vout = tx.vout[utxoSeal.vout]; | ||
| if (!vout) { | ||
| throw new ErrorBtcUtxoNotFound( | ||
| utxoSeal.txid, | ||
| utxoSeal.vout, | ||
| tx.vout.length, | ||
| ); | ||
| } | ||
|
|
||
| const scriptBuffer = ccc.bytesFrom(vout.scriptpubkey); | ||
| if (isOpReturnScriptPubkey(scriptBuffer)) { | ||
| throw new ErrorBtcOpReturnUtxo(utxoSeal.txid, utxoSeal.vout); | ||
| } | ||
|
|
||
| inputs.push( | ||
| await utxoToInputData( | ||
| { | ||
| txid: utxoSeal.txid, | ||
| vout: utxoSeal.vout, | ||
| value: vout.value, | ||
| scriptPk: vout.scriptpubkey, | ||
| address: vout.scriptpubkey_address, | ||
| addressType: getAddressType(vout.scriptpubkey_address), | ||
| } as Utxo, | ||
| this.publicKeyProvider, | ||
| ), | ||
| ); | ||
| } | ||
| return inputs; |
There was a problem hiding this comment.
The loop to build inputs makes sequential await calls inside for...of. This can be inefficient if there are many UTXOs to process. You can improve performance by processing them in parallel using Promise.all. The existing // TODO: parallel comment indicates this was intended.
const inputs: TxInputData[] = await Promise.all(
uniqueSeals.map(async (utxoSeal) => {
const tx = await this.dataSource.getTransaction(utxoSeal.txid);
if (!tx) {
throw new ErrorBtcTransactionNotFound(utxoSeal.txid);
}
const vout = tx.vout[utxoSeal.vout];
if (!vout) {
throw new ErrorBtcUtxoNotFound(
utxoSeal.txid,
utxoSeal.vout,
tx.vout.length,
);
}
const scriptBuffer = ccc.bytesFrom(vout.scriptpubkey);
if (isOpReturnScriptPubkey(scriptBuffer)) {
throw new ErrorBtcOpReturnUtxo(utxoSeal.txid, utxoSeal.vout);
}
return utxoToInputData(
{
txid: utxoSeal.txid,
vout: utxoSeal.vout,
value: vout.value,
scriptPk: vout.scriptpubkey,
address: vout.scriptpubkey_address,
addressType: getAddressType(vout.scriptpubkey_address),
} as Utxo,
this.publicKeyProvider,
);
}),
);
return inputs;| return Math.ceil( | ||
| bufferedVirtualSize * (feeRate ?? this.networkConfig.btcFeeRate), | ||
| ); |
There was a problem hiding this comment.
The feeRate variable is guaranteed to be a number at this point due to the preceding logic that assigns a value in all branches. The nullish coalescing operator ?? is therefore redundant.
return Math.ceil(bufferedVirtualSize * feeRate);| console.log( | ||
| `[prepareUtxoSeal] Transaction ${txId} not confirmed, waiting ${intervalSeconds} seconds...`, | ||
| ); |
There was a problem hiding this comment.
Using console.log for logging in a library is generally discouraged as it can create unwanted output in consuming applications and is not configurable. Consider using a dedicated, configurable logger or providing an option to suppress these logs.
| const outputDataLen = ccc.numLeToBytes(ccc.bytesFrom(outputData).length, 4); | ||
| const od = ccc.bytesFrom(outputData); | ||
| hash.update(outputDataLen); | ||
| hash.update(od); |
There was a problem hiding this comment.
The expression ccc.bytesFrom(outputData) is evaluated twice. To improve performance and readability, store its result in a variable. Additionally, ensure the function accepts generic types like HexLike for outputData and performs this conversion internally, consistent with repository standards.
| const outputDataLen = ccc.numLeToBytes(ccc.bytesFrom(outputData).length, 4); | |
| const od = ccc.bytesFrom(outputData); | |
| hash.update(outputDataLen); | |
| hash.update(od); | |
| const od = ccc.bytesFrom(outputData); | |
| const outputDataLen = ccc.numLeToBytes(od.length, 4); | |
| hash.update(outputDataLen); | |
| hash.update(od); |
References
- Functions should accept more generic types like HexLike and perform necessary type conversions internally, rather than requiring the caller to do so.
| setTimeout(() => { | ||
| if (this.spvProofCache.get(btcTxId) === proofPromise) { | ||
| this.spvProofCache.delete(btcTxId); | ||
| } | ||
| }, this.cacheExpiryTime); |
There was a problem hiding this comment.
The timer created with setTimeout can prevent a Node.js process from exiting if it is the only thing left in the event loop. For a library, it is good practice to call .unref() on the timer handle to allow the process to exit gracefully even if the timer is still pending.
| setTimeout(() => { | |
| if (this.spvProofCache.get(btcTxId) === proofPromise) { | |
| this.spvProofCache.delete(btcTxId); | |
| } | |
| }, this.cacheExpiryTime); | |
| setTimeout(() => { | |
| if (this.spvProofCache.get(btcTxId) === proofPromise) { | |
| this.spvProofCache.delete(btcTxId); | |
| } | |
| }, this.cacheExpiryTime).unref(); |
…ansaction building
…+ waitForConfirmation
3 participants
No description provided.