Skip to content

fix: return transaction objects on repeated bridge swaps - #4

Open
newmattock wants to merge 1 commit into
BitgesellOfficial:mainfrom
newmattock:codex/wbgl-transaction-factory-reuse
Open

fix: return transaction objects on repeated bridge swaps#4
newmattock wants to merge 1 commit into
BitgesellOfficial:mainfrom
newmattock:codex/wbgl-transaction-factory-reuse

Conversation

@newmattock

Copy link
Copy Markdown

Summary

  • Fix Transaction.makeTxObject() so it returns a fresh Bitgesell transaction object on every call, not only during the first SDK initialization.
  • Add a focused regression test proving repeated factory calls no longer return undefined.

Verification

  • pnpm exec jest src/__tests__/Transaction.test.ts --runInBand
  • pnpm exec tsc --noEmit --skipLibCheck --module commonjs --target es2019 --esModuleInterop --types jest src/bridge/Transaction.ts src/__tests__/Transaction.test.ts
  • pnpm exec prettier --check src/bridge/Transaction.ts src/__tests__/Transaction.test.ts
  • git diff --check

Notes

  • pnpm exec tsc -p tsconfig.json --skipLibCheck currently fails on upstream because the repo's tsconfig.json references the node type package but @types/node is not declared as a visible dependency in this install. The focused compile command above covers the changed source and regression test.
  • pnpm exec eslint src/bridge/Transaction.ts src/__tests__/Transaction.test.ts --ext .ts did not complete locally after roughly 45 seconds and was stopped.

Bounty context

Submitted for Bitgesell improvement bounty consideration: BitgesellOfficial/bitgesell#39.

@MyTH-zyxeon

Copy link
Copy Markdown

Review-assist note for the Bitgesell #81 improvement lane.

I checked the public diff and repo test config for this PR:

  • Scope is narrow: Transaction.makeTxObject() now initializes jsbgl once but returns a fresh globalThis.Transaction on every call, which covers the repeated-call case instead of returning undefined after the first initialization path.
  • Regression coverage is present in src/__tests__/Transaction.test.ts; the repo's Jest config uses roots: ['src/__tests__/'], so the new test should be discoverable by the normal Jest path.
  • No bridge secrets, RPC endpoint changes, signing-path changes, or live transaction execution are introduced; the patch stays in the factory/test layer.

One maintainer-side check I would suggest before merge: run the repo's normal Jest suite, or at least the new transaction factory test, to confirm the mocked @naftalimurgor/jsbgl import behaves the same in CI. From the public patch shape, this looks like a small, reviewable fix.

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.

2 participants