Skip to content

fix(dex - Certik): schedule both GoodTil delays when height and time are set#137

Open
metalarm10 wants to merge 3 commits into
masterfrom
john/certik-good-til-expiry-fix
Open

fix(dex - Certik): schedule both GoodTil delays when height and time are set#137
metalarm10 wants to merge 3 commits into
masterfrom
john/certik-good-til-expiry-fix

Conversation

@metalarm10
Copy link
Copy Markdown
Contributor

@metalarm10 metalarm10 commented May 29, 2026

Closes: https://app.clickup.com/t/868jnp17x

Summary

delayGoodTilCancellation used an if/return chain that registered only the height-based delay when both GoodTilBlockHeight and GoodTilBlockTime were set on the same order. Orders then remained matchable past their declared GoodTilBlockTime until GoodTilBlockHeight was reached.

Replaced with parallel if blocks matching removeGoodTilDelay's existing structure. Both delays are now scheduled when both fields are set; whichever fires first cancels the order, and the second cleanup becomes a no-op against the missing KV key.

Refs: CertiK Submission no:2 - accepted as Low severity.

Test & Verification

  • New regression test no_match_with_good_til_block_time_before_height_cancel_by_time exercises the time-before-height case (time expires at block 103, height set to 200, endHeight=105 — order can only be canceled via the time-based path).
  • Full TestKeeper_GoodTil suite passes.
  • Verified the new test fails without the fix (via git stash of good_til.go) — confirms it specifically guards against this regression and isn't a false positive.

Description

Reviewers checklist:

  • Try to write more meaningful comments with clear actions to be taken.
  • Nit-picking should be unblocking. Focus on core issues.

Authors checklist

  • Provide a concise and meaningful description
  • Review the code yourself first, before making the PR.
  • Annotate your PR in places that require explanation.
  • Think and try to split the PR to smaller PR if it is big.

This change is Reviewable

@metalarm10 metalarm10 marked this pull request as ready for review May 29, 2026 13:16
@metalarm10 metalarm10 requested a review from a team as a code owner May 29, 2026 13:16
@metalarm10 metalarm10 requested review from bashash, grainerycafe and ysv and removed request for a team May 29, 2026 13:16
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