-
Notifications
You must be signed in to change notification settings - Fork 31
use ts-sdk #267
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
base: master
Are you sure you want to change the base?
use ts-sdk #267
Conversation
|
Just curious, this PR is intended to replace the sCrypt-Inc/bsv dependency with the bitcoin-sv/ts-sdk dependency, correct? Is it still on going? |
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.
Pull Request Overview
This PR refactors the codebase to use a new TypeScript SDK abstraction layer instead of directly using the bsv library. The main purpose is to introduce a chain abstraction through a factory pattern that enables support for multiple blockchain implementations while maintaining compatibility with existing code.
- Introduces a new Chain factory pattern with BSV implementation
- Removes direct bsv library dependencies and replaces with abstracted interfaces
- Updates test files to use the new SDK patterns and transaction handling
- Migrates utility functions to use the new chain abstraction layer
Reviewed Changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/utils.test.ts | Removes int2Asm function tests and updates imports |
| test/tx.test.ts | New test file demonstrating SDK usage patterns |
| test/helper.ts | Updates transaction creation to use Chain factory |
| test/counter.test.ts | Migrates test to use new transaction and verification patterns |
| test/accumulatorMultiSig.test.ts | Updates to use SDK primitives for key generation |
| test/abi.test.ts | Converts to Chain factory for cryptographic operations |
| src/utils.ts | Major refactor to use Chain abstraction instead of bsv directly |
| src/stateful.ts | Updates to use Chain factory for script and reader operations |
| src/serializer.ts | Migrates to Chain factory for script operations |
| src/launchConfig.ts | Adds type casting for bigint comparison |
| src/index.ts | Removes exported utility functions and constants |
| src/deserializer.ts | Updates to use Chain factory for script parsing |
| src/contract.ts | Major refactor to use Chain abstraction for transaction verification |
| src/compilerWrapper.ts | Updates hash function encoding parameter |
| src/chain/* | New abstraction layer with BSV implementation |
| src/builtins.ts | Updates to use Chain factory for cryptographic operations |
| src/abi.ts | Migrates to Chain factory for script operations |
| package.json | Updates dependencies to use @bsv/sdk instead of bsv |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| const key = factory.PrivateKey.fromRandom(); | ||
| // tx.addInput() | ||
| console.log(key.toAddress()) |
Copilot
AI
Aug 15, 2025
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 file appears to be a temporary development/debugging file with console.log statements and incomplete test structure. Consider removing this file or converting it into proper unit tests with assertions.
| console.log(key.toAddress()) | |
| import * as assert from "assert"; | |
| describe("Chain Factory and Script Tests", function () { | |
| it("should create and manipulate LockingScript correctly", function () { | |
| const factory = Chain.getFactory(); | |
| const s = factory.LockingScript.fromHex('76a914212771cc264264057238cc3b98a03ddd9aa3a31c88ac'); | |
| s.writeNumber(3).writeBn(BigInt(30)); | |
| s.writeOpCode(OP.OP_2SWAP); | |
| const ss = factory.UnlockingScript.fromHex('020111'); | |
| s.writeScript(ss); | |
| // Check that s.toASM() returns a non-empty string | |
| assert.ok(typeof s.toASM() === "string" && s.toASM().length > 0, "ASM should be a non-empty string"); | |
| }); | |
| it("should create Transaction from hex", function () { | |
| const factory = Chain.getFactory(); | |
| const tx = factory.Transaction.fromHex('01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff1a034fcd0c2f7461616c2e636f6d2ffd54e6dd4fe37955f2600000ffffffff0161cf4025000000001976a914522cf9e7626d9bd8729e5a1398ece40dad1b6a2f88ac00000000'); | |
| assert.ok(tx, "Transaction should be created"); | |
| }); | |
| it("should create empty Transaction", function () { | |
| const factory = Chain.getFactory(); | |
| const tx1 = factory.Transaction.from(); | |
| assert.ok(tx1, "Empty Transaction should be created"); | |
| }); | |
| it("should generate a random PrivateKey and get its address", function () { | |
| const factory = Chain.getFactory(); | |
| const key = factory.PrivateKey.fromRandom(); | |
| const address = key.toAddress(); | |
| assert.ok(typeof address === "string" && address.length > 0, "Address should be a non-empty string"); | |
| }); | |
| }); |
|
|
||
| console.log('result', result) | ||
|
|
||
| let callTx = new Transaction() |
Copilot
AI
Aug 15, 2025
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.
The variable callTx is declared but Transaction is not imported. This will cause a runtime error since Transaction is undefined in this scope.
| })); | ||
|
|
||
| const abn = unpack(this.flattenSha256(a[0], keyType)); | ||
| const bbn = unpack(this.flattenSha256(b[0], keyType)); |
Copilot
AI
Aug 15, 2025
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.
Accessing a[0] in this context appears incorrect. The variable a is the first element of a map entry, so it should be a instead of a[0]. This pattern is repeated on multiple lines.
| const bbn = unpack(this.flattenSha256(b[0], keyType)); | |
| const abn = unpack(this.flattenSha256(a, keyType)); | |
| const bbn = unpack(this.flattenSha256(b, keyType)); |
| })); | ||
|
|
||
| const abn = unpack(this.flattenSha256(a[0], keyType)); | ||
| const bbn = unpack(this.flattenSha256(b[0], keyType)); |
Copilot
AI
Aug 15, 2025
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.
Same issue as above - accessing a[0] when a is already the key element. Should be a instead of a[0].
| const bbn = unpack(this.flattenSha256(b[0], keyType)); | |
| const abn = unpack(this.flattenSha256(a, keyType)); | |
| const bbn = unpack(this.flattenSha256(b, keyType)); |
| export function pack(n: bigint): Bytes { | ||
| const num = new bsv.crypto.BN(n); | ||
| return num.toSM({ endian: 'little' }).toString('hex'); | ||
| return num2bin(n); |
Copilot
AI
Aug 15, 2025
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.
The function calls num2bin(n) but num2bin is not imported or defined in this scope. This will cause a runtime error.
| */ | ||
| export function unpack(a: Bytes): bigint { | ||
| return BigInt(bin2num(a)); | ||
| return bin2num(a); |
Copilot
AI
Aug 15, 2025
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.
The function calls bin2num(a) but bin2num is not imported or defined in this scope. This will cause a runtime error.
| return bin2num(a); | |
| return bin2num(toHex(a)); |
No description provided.