Skip to content

fix: await and reserve bridge service nonces#40

Open
Chillzyg wants to merge 1 commit into
BitgesellOfficial:mainfrom
Chillzyg:codex/service-nonce-reservation
Open

fix: await and reserve bridge service nonces#40
Chillzyg wants to merge 1 commit into
BitgesellOfficial:mainfrom
Chillzyg:codex/service-nonce-reservation

Conversation

@Chillzyg
Copy link
Copy Markdown

@Chillzyg Chillzyg commented Jun 2, 2026

Summary

  • await service nonce initialization before processing confirmed BGL deposits
  • reserve the current ETH/BSC nonce before incrementing the local counter so the first available nonce is not skipped
  • accept the documented ETH_NONCE environment variable while preserving the existing ETH__NONCE fallback
  • add focused nonce helper coverage that does not require live bridge services

Why

setupNonce() called async getTransactionCount() without awaiting it. On a cold service start, ethNonce / bscNonce could become a Promise; the later increment path then risked passing an invalid nonce into sendWBGL().

Verification

  • npm install --ignore-scripts --package-lock=false in service/
  • npx mocha src/tests/nonce.js --timeout 10000 in service/
  • npx prettier --check src/utils/nonce.js src/utils/config.js src/modules/chores.js src/tests/nonce.js in service/
  • node --check src/utils/nonce.js && node --check src/utils/config.js && node --check src/modules/chores.js && node --check src/tests/nonce.js in service/
  • npx eslint --parser-options '{"ecmaVersion":2022,"sourceType":"module"}' src/utils/nonce.js src/utils/config.js src/modules/chores.js src/tests/nonce.js in service/
  • git diff --check

Note: plain npx eslint ... fails before analysis in this repo because the current ESLint config does not parse the service's ES-module import/export syntax.

Submitted for Bitgesell improvement bounty consideration: BitgesellOfficial/bitgesell#39. Payment details can be provided if accepted.

@MyTH-zyxeon
Copy link
Copy Markdown

Maintainer review assist for Bitgesell bounty routing (#81 / #39): this looks like a clean, focused bridge-service correctness fix rather than a broad refactor.

Why this PR seems materially useful:

  • setupNonce() now awaits the chain nonce instead of leaving ethNonce / bscNonce as unresolved Promises on cold start.
  • The send path now reserves the current nonce first and only then increments the local counter, so the first usable nonce is not skipped.
  • Config parsing now accepts the documented ETH_NONCE while preserving the older ETH__NONCE fallback, which reduces env-name drift risk.
  • Focused helper tests cover configured parsing, initial nonce resolution, and the reserve-before-increment behavior without requiring live bridge services.

If you are deciding whether to merge/pay this under the improvement program, the key acceptance checks seem to be:

  • cold-start deposit processing no longer risks passing a Promise/invalid nonce into sendWBGL()
  • the first outbound tx after startup uses the fetched/configured nonce itself, not nonce + 1
  • existing deployments using ETH__NONCE do not break while documented ETH_NONCE now works
  • the new test scope is narrow and matches the behavior change

From the public metadata, this PR is currently OPEN + MERGEABLE and had no prior PR comments/reviews visible when checked from my side.

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