Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 22, 2025

Code review feedback identified redundant trim logic and confusing error return patterns in the proxy chain resolution code.

Changes:

  • Centralized trimming: Move strings.TrimSpace into getPropertyValuesAt so all on-chain property values are trimmed at the source
  • Removed redundant trim logic: Delete custom strings.IndexAny(proxyTo, " \t\r\n") implementation and duplicate strings.TrimSpace call from buildProxyToChain
  • Clarified intent: Replace fetchErr with explicit nil returns where buildProxyToChain completes successfully (empty proxy_to, invalid address, loop detection)

Before:

proxyTo := strings.TrimSpace(props["proxy_to"])
if proxyTo == "" {
    return chain, fetchErr  // confusing: fetchErr may be nil
}
if idx := strings.IndexAny(proxyTo, " \t\r\n"); idx >= 0 {
    proxyTo = proxyTo[:idx]  // redundant custom trim
}

After:

proxyTo := props["proxy_to"]  // already trimmed by getPropertyValuesAt
if proxyTo == "" {
    return chain, nil  // explicit: successful completion
}

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.


Note

Trim property values in getPropertyValuesAt and simplify buildProxyToChain by removing redundant whitespace handling and returning nil on normal termination.

  • Join contract handling (cmd/diode/join.go):
    • Centralized trimming: Trim unpacked on-chain values in getPropertyValuesAt.
    • Proxy chain resolution: Simplify buildProxyToChain by using already-trimmed proxy_to, removing custom whitespace parsing, and returning (chain, nil) on normal completion cases (empty/same/invalid/loop).

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

Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix proxy_to redirect chains for contract sync Centralize property value trimming and clarify error returns in buildProxyToChain Dec 22, 2025
Copilot AI requested a review from dominicletz December 22, 2025 09:58
@dominicletz dominicletz marked this pull request as ready for review December 22, 2025 10:03
@dominicletz dominicletz merged commit 9b3b274 into tuhalf/support-join-command-for-perimeter-redirect Dec 22, 2025
@dominicletz dominicletz deleted the copilot/sub-pr-202 branch December 22, 2025 10: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 January 10

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

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

if cfg != nil && cfg.Logger != nil {
cfg.Logger.Warn("Detected proxy_to loop at %s; stopping resolution", proxyTo)
}
return chain, fetchErr
Copy link

Choose a reason for hiding this comment

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

Inconsistent return uses unassigned named variable instead of nil

The PR's stated goal is to replace implicit error returns with explicit nil for clarity. Lines 269, 272, 279, and 287 were updated to return nil, but line 297 still returns the named variable err which is never assigned in the function. While functionally equivalent to nil, this is inconsistent with the pattern established by the other changes in this PR and could confuse future maintainers who expect the variable to hold a meaningful value.

Fix in Cursor Fix in Web

dominicletz added a commit that referenced this pull request Dec 22, 2025
* join: follow proxy_to perimeters for contract redirect

* Adjusted proxy_to logging so Perimeter Proxy Chain/Effective Perimeter only print when the effective perimeter changes

* Centralize property value trimming and clarify error returns in buildProxyToChain (#204)

* Initial plan

* Refactor trim logic and fix error returns in buildProxyToChain

Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: dominicletz <2987674+dominicletz@users.noreply.github.com>
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