Skip to content

Conversation

@zhfnjust
Copy link
Contributor

No description provided.

@zhfnjust zhfnjust changed the title Remove bsv use ts-sdk Apr 10, 2024
@YinanDanielZhou
Copy link

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?

@freedomhero freedomhero requested a review from Copilot August 15, 2025 10:39
Copy link

Copilot AI left a 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())
Copy link

Copilot AI Aug 15, 2025

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.

Suggested change
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");
});
});

Copilot uses AI. Check for mistakes.

console.log('result', result)

let callTx = new Transaction()
Copy link

Copilot AI Aug 15, 2025

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.

Copilot uses AI. Check for mistakes.
}));

const abn = unpack(this.flattenSha256(a[0], keyType));
const bbn = unpack(this.flattenSha256(b[0], keyType));
Copy link

Copilot AI Aug 15, 2025

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.

Suggested change
const bbn = unpack(this.flattenSha256(b[0], keyType));
const abn = unpack(this.flattenSha256(a, keyType));
const bbn = unpack(this.flattenSha256(b, keyType));

Copilot uses AI. Check for mistakes.
}));

const abn = unpack(this.flattenSha256(a[0], keyType));
const bbn = unpack(this.flattenSha256(b[0], keyType));
Copy link

Copilot AI Aug 15, 2025

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].

Suggested change
const bbn = unpack(this.flattenSha256(b[0], keyType));
const abn = unpack(this.flattenSha256(a, keyType));
const bbn = unpack(this.flattenSha256(b, keyType));

Copilot uses AI. Check for mistakes.
export function pack(n: bigint): Bytes {
const num = new bsv.crypto.BN(n);
return num.toSM({ endian: 'little' }).toString('hex');
return num2bin(n);
Copy link

Copilot AI Aug 15, 2025

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.

Copilot uses AI. Check for mistakes.
*/
export function unpack(a: Bytes): bigint {
return BigInt(bin2num(a));
return bin2num(a);
Copy link

Copilot AI Aug 15, 2025

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.

Suggested change
return bin2num(a);
return bin2num(toHex(a));

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants