Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

Extract the retry-with-gas-estimation pattern duplicated between `submitter.service.ts` and `submission-batch.service.ts`

Open
  • Is something wrong?
#211 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
25/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
typescript
Domain
backend, blockchain

Research direction

Start by comparing the retry loops in src/blockchain/oracle/services/submitter.service.ts (around line 235) and src/blockchain/oracle/services/submission-batch.service.ts (around line 216), and settle the maxRetries meaning with maintainers before writing code. Done means both services call one shared helper placed under src/common/utils/, the per-attempt findOne is hoisted, and new specs cover retry, terminal failure, and no duplicate submission. Model the spec on payload-signing.service.spec.ts.

Written by the indexing model from the issue text.

Description

advanced help wanted refactor

Problem

This is the duplication-specific companion to the larger submitter-service split. The same retry loop shape appears in two of the largest files in the repository:

src/blockchain/oracle/services/submitter.service.ts:235

for (let attempt = 1; attempt <= this.maxRetries + 1; attempt++) {
  const gasEstimate = await this.estimateGas(payload);
  const gasLimit = BigInt(Math.ceil(Number(gasEstimate) * this.gasLimitMultiplier));
  ...
}

src/blockchain/oracle/services/submission-batch.service.ts:216

const attempts = maxRetries ?? this.maxRetries;
for (let attempt = 1; attempt <= attempts; attempt++) {
  const payload = await this.payloadRepository.findOne({ where: { id: payloadId } });
  ...
}

Note the loop bounds differ — <= this.maxRetries + 1 versus <= maxRetries ?? this.maxRetries. Whether that is intentional or a bug is unclear from the code, which is itself the problem: retry semantics are a financial-correctness concern and should be stated once.

Why it matters

Duplicated retry logic diverges. If the batch path is corrected for a bug and the single path is not, or a backoff policy is added to one, the two behave differently — and on-chain submission is exactly where divergent behaviour costs money. The per-attempt findOne in the batch loop is also a database round-trip that repeats work the caller may have already done.

Suggested Direction

  1. Extract a shared retry helper with an explicit contract:

    retry<T>(fn: (attempt: number) => Promise<T>, opts: {
  maxAttempts: number;
  shouldRetry: (err: unknown) => boolean;
  backoff?: (attempt: number) => number;
  onRetry?: (err: unknown, attempt: number) => void;
}): Promise<T>

    Check src/common/utils/ for where similar shared helpers already live and follow that placement.

  2. Resolve the off-by-one discrepancy. Determine whether maxRetries means "retry attempts" or "total attempts", then use the same meaning in both call sites. Raise it with maintainers if the intent is unclear — do not silently pick one.

  3. Hoist the idempotency check out of the batch retry loop; fetch once before looping.

  4. Pin the behaviour with tests — this currently has none. Cover: success first try, success after N failures, terminal failure, and the assertion that a failure never produces a duplicate submission.

Constraints

  • Idempotency must be preserved or strengthened. Never retry a write to the chain without re-checking submission state.
  • Retry counts and backoff are operational tuning parameters — keep them externally configurable via config, not literals.
  • There is an existing src/blockchain/oracle/services/payload-signing.service.spec.ts to follow for the colocated-spec convention.

Acceptance Criteria

  • A shared retry helper exists in a common location and both services use it.
  • The maxRetries off-by-one discrepancy is resolved and documented.
  • The per-attempt findOne is hoisted out of the retry loop.
  • Tests cover retry, terminal failure, and the no-duplicate-submission guarantee.
  • npx tsc --noEmit and npm test pass.
Dominant language
TypeScript
Stars
7
Forks
55
Avg merge
1d 17h
Merged PRs (30d)
5

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from SourceXXL/alian_structure-api

All issues in SourceXXL/alian_structure-api

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.