Skip to content

Conversation

@Vishalkulkarni45
Copy link
Collaborator

@Vishalkulkarni45 Vishalkulkarni45 commented Oct 1, 2025

Note

Updates Go verifier to mark OFAC valid if any check passes and stops flagging OFAC issues when OFAC is disabled in config.

  • Go SDK (sdk/sdk-go/verifier.go):
    • OFAC verification:
      • Compute cumulativeOfac (true if any Ofac check is true) and set IsOfacValid from it when verificationConfig.Ofac is enabled.
      • Remove config validation that appended InvalidOfac issues when Ofac is disabled.

Written by Cursor Bugbot for commit 4a01df3. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • Bug Fixes

    • Improved OFAC validation to aggregate results across checks for more accurate pass/fail outcomes.
    • Respects configuration flags, preventing incorrect OFAC invalidations when OFAC verification is disabled.
    • Ensures issues are collected earlier, resulting in clearer, more consistent verification results.
  • Refactor

    • Streamlined internal validation flow to reduce redundancy and improve maintainability, with no changes to public APIs.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

The OFAC verification logic in sdk/sdk-go/verifier.go was refactored to compute a cumulative OFAC result and derive isOfacValid based on configuration and prior config errors. The prior per-check issue appends for OFAC were removed. genericDiscloseOutput is computed once and reused across config and OFAC validation.

Changes

Cohort / File(s) Summary
OFAC validation refactor
sdk/sdk-go/verifier.go
Replaced per-check OFAC boolean loop with cumulativeOfac and derived isOfacValid gated by configErr and verificationConfig.Ofac; removed per-check InvalidOfac issue appends; computed and reused genericDiscloseOutput before validations.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Caller
    participant Verifier
    participant Config as VerificationConfig
    participant Circuit
    participant Issues as IssueCollector

    Caller->>Verifier: Verify(...)
    Verifier->>Circuit: compute genericDiscloseOutput
    Verifier->>Issues: collect config validation issues
    Verifier->>Verifier: configErr = hasIssues(Issues)

    Verifier->>Circuit: iterate OFAC checks
    Note over Verifier,Circuit: Accumulate results into cumulativeOfac
    Verifier->>Verifier: isOfacValid = (!configErr) AND Config.Ofac AND cumulativeOfac

    alt isOfacValid
        Verifier->>Caller: return success (no OFAC issues)
    else OFAC invalid
        Verifier->>Issues: append OFAC issue once (based on cumulative result)
        Verifier->>Caller: return with issues
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • fix: ofac logic #1124 — Implements the same OFAC validation consolidation using a cumulative flag in a parallel code path, indicating coordinated refactor of OFAC checks.

Poem

Flags once scattered, now gathered as one,
A single sum where many had run.
The circuit whispers, configs agree,
OFAC speaks cumulatively.
Issues collected, the path made clear—
One tidy verdict, nothing to fear.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the main change of aligning the OFAC check logic in the Go SDK to match the TypeScript SDK, and it directly reflects the core objective of the pull request in a clear and specific manner.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/go-sdk-ofac

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6468ee9 and 4a01df3.

📒 Files selected for processing (1)
  • sdk/sdk-go/verifier.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/sdk-go/verifier.go (1)
sdk/sdk-go/utils.go (1)
  • Ofac (90-90)
🔇 Additional comments (1)
sdk/sdk-go/verifier.go (1)

463-476: Verify TypeScript SDK OFAC logic consistency
Ripgrep didn’t find any OFAC validation in the TS SDK here; please manually confirm that the Go SDK’s isOfacValid behavior—particularly when OFAC is not required—matches the TypeScript SDK implementation.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gitguardian
Copy link

gitguardian bot commented Oct 1, 2025

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@Vishalkulkarni45 Vishalkulkarni45 changed the base branch from main to dev October 1, 2025 08:03
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on October 17

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

isOfacValid := false
if configErr == nil && verificationConfig.Ofac {
isOfacValid = cumulativeOfac
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: OFAC Validation Default Behavior Changed

The isOfacValid variable now defaults to false when OFAC checks are not required by the configuration or if a config error exists. This changes the previous behavior where it defaulted to true, causing IsOfacValid to incorrectly report false even when OFAC validation isn't applicable.

Fix in Cursor Fix in Web

Copy link
Collaborator

@Nesopie Nesopie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Nesopie Nesopie merged commit 9131cd3 into dev Oct 1, 2025
13 of 14 checks passed
@Nesopie Nesopie deleted the fix/go-sdk-ofac branch October 1, 2025 10:01
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.

3 participants