Skip to content

Fix exact project escrow ledger scoping#210

Open
yyswhsccc wants to merge 1 commit into
mergeos-bounties:masterfrom
yyswhsccc:fix/escrow-exact-ledger-scope
Open

Fix exact project escrow ledger scoping#210
yyswhsccc wants to merge 1 commit into
mergeos-bounties:masterfrom
yyswhsccc:fix/escrow-exact-ledger-scope

Conversation

@yyswhsccc

@yyswhsccc yyswhsccc commented Jun 5, 2026

Copy link
Copy Markdown

Claim

Description

Project escrow summaries could treat sibling ledger references such as prj_0010 or tsk_0010 as if they belonged to prj_001 when reserve rows were matched by loose prefixes.

This PR reuses the existing exact ledger ID boundary matcher for project escrow reserve rows, so only exact project/task references are included in the summary.

Evidence

Before:

  • Project escrow summary scope could include sibling project/task rows whose IDs shared the same prefix.

After:

  • prj_001 reserve/release summaries are constrained to exact ID boundaries and exclude sibling prj_0010 / tsk_0010 rows.

Additional logs or test output:

  • go test ./internal/core -run 'TestProjectEscrowLedgerAppliesUsesExactIDBoundaries|TestProjectEscrowRouteReturnsReserveReleaseSummary' -> ok mergeos/backend/internal/core 0.670s
  • Current GitHub Backend build is blocked by the shared Go 1.25.10 govulncheck baseline; Bump backend Go patch version #218 updates the backend Go patch version and is green. Secret scan, web checks, and MergeIDE are passing on this PR.

Safety

  • Reporting/scoping only.
  • Does not change payout amounts, payout rules, manual credits, production wallets, or secrets.

Tests

  • I ran the relevant tests.
  • I included the command output or explained why tests were not run.

Bounty Checklist

  • I starred this repository before claiming or starting bounty work.
  • This PR links to a confirmed Claim Token comment.
  • This PR includes image evidence for UI bugs or logs/test output for non-UI bugs.
  • This PR explains the behavior before and after the fix.
  • This PR does not expose secrets, private keys, customer data, or sensitive production details.

@TUPM96

TUPM96 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Findings
No blocking code issues. The change correctly replaces a loose string containment check with an exact ID boundary matcher (ledgerEntryReferencesID) for project escrow ledger entries. This resolves the described bug where sibling ledger references could be incorrectly included. The new test TestProjectEscrowLedgerAppliesUsesExactIDBoundaries effectively validates the fix and covers the described edge cases.

Bounty Readiness

  • **Blocking

MergeOS automated readiness signals:

  • Evidence signal: evidence: missing
  • Repository star: star: verified

@TUPM96 TUPM96 added evidence: missing PR needs screenshot, GIF, video, or other visual evidence. star: verified PR author has starred this repository. labels Jun 5, 2026
@yyswhsccc

Copy link
Copy Markdown
Author

Maintenance note: I checked the failing Backend build and test job on this PR. The failure is the shared base/toolchain govulncheck issue from backend/go.mod using Go 1.25.10, not this PR's local test suite. govulncheck reports standard-library issues GO-2026-5039 and GO-2026-5037 fixed in Go 1.25.11 before project tests run.

I opened #218 to bump backend/go.mod to Go 1.25.11. #218 is now green across Backend build and test, Secret scan, frontend/admin/scan web checks, and MergeIDE. After that toolchain patch lands and this PR is updated against it, the backend check should no longer fail on the Go 1.25.10 standard-library findings.

@yZangEren

Copy link
Copy Markdown

MergeOS PR #210 Verification Report

PR: #210
Verifier: independent QA reviewer
Checked at: 2026-06-06 11:40 +08:00
Head SHA: 2db793a7738616b477a789d9d63fd96639cfac3c

Verdict

Approve from code review and local test evidence. I found no blocking issue in the exact project escrow ledger scoping change.

Scope Reviewed

Changed files:

  • backend/internal/core/escrow.go
  • backend/internal/core/store_test.go

The patch replaces a loose string containment check:

strings.Contains(haystack, projectID)

with the existing exact ledger ID boundary helper:

ledgerEntryReferencesID(entry, projectID)

That is the right behavior for the reported bug. A project such as prj_001 should not include reserve/release rows for sibling IDs like prj_0010, and task payment/manual credit rows should already be scoped through the task ID map.

Local Checks

Diff scope:

git diff --name-only origin/master...HEAD
backend/internal/core/escrow.go
backend/internal/core/store_test.go

Whitespace check:

git diff --check origin/master...HEAD
no output, exit 0

Go version used for local test:

go version go1.25.11 windows/amd64

Targeted backend test:

go test ./internal/core -run 'TestProjectEscrowLedgerAppliesUsesExactIDBoundaries|TestProjectEscrowRouteReturnsReserveReleaseSummary'

Result:

ok  	mergeos/backend/internal/core	2.912s

Test Coverage Review

The new test covers the exact edge cases needed for the bug:

  • exact project reserve rows for prj_001 are included;
  • sibling project reserve rows for prj_0010 are excluded;
  • sibling task references such as tsk_0010 are excluded when only tsk_001 belongs to the project.

The existing escrow route test still verifies the reserve/release summary and authorization behavior, so the targeted fix is covered without expanding the scope into payout or wallet logic.

CI Review

Current PR checks:

  • Secret scan: success
  • Web build and test (frontend): success
  • Web build and test (admin): success
  • Web build and test (scan): success
  • MergeIDE task runner: success
  • Backend build and test: failure

The backend failure is the shared Go toolchain baseline issue seen on nearby PRs. The log shows go1.25.10, then govulncheck, then standard-library findings:

  • GO-2026-5039, found in net/textproto@go1.25.10, fixed in go1.25.11
  • GO-2026-5037, found in crypto/x509@go1.25.10, fixed in go1.25.11

Because PR #210 changes escrow ledger scoping and tests only, and the targeted local tests pass under Go 1.25.11, I do not treat this backend CI failure as caused by the patch.

Recommendation

Approve/accept after maintainer review. No required code changes from my pass.

Payout address for this QA verification if accepted under #64:

  • Preferred: github:yZangEren
  • Fallback public wallet: Solana 61yLtKuSbiGrVEJDR1NDY9KpUP7XXuRv3oSZF35kH3hR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

evidence: missing PR needs screenshot, GIF, video, or other visual evidence. star: verified PR author has starred this repository.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants