Skip to content

Remove deprecated accepted route #514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

BelfordZ
Copy link
Contributor

@BelfordZ BelfordZ commented May 22, 2025

User description

Summary

  • add enableAcceptedRoute config flag
  • gate old accepted route behind the flag
  • remove unused acceptance notification code
  • document the deprecated /accepted endpoint

Testing

  • npm test

PR Type

Enhancement, Documentation


Description

  • Add enableAcceptedRoute config flag to control deprecated route

  • Remove unused acceptance notification code and logic

  • Gate /accepted route behind new config flag for legacy support

  • Update documentation to reflect deprecation of /accepted endpoint


Changes walkthrough 📝

Relevant files
Enhancement
server.ts
Add config flag to control deprecated `/accepted` route   

src/config/server.ts

  • Add enableAcceptedRoute flag to server configuration.
  • Default the flag to false.
  • +1/-0     
    routes.ts
    Gate `/accepted` route behind config flag                               

    src/p2p/Join/routes.ts

  • Update documentation for deprecated /accepted route.
  • Gate inclusion of acceptedRoute behind enableAcceptedRoute config
    flag.
  • +12/-3   
    index.ts
    Remove deprecated acceptance notification logic                   

    src/p2p/Join/v2/index.ts

  • Remove commented and deprecated acceptance notification logic.
  • Add comments clarifying removal of old /accepted route.
  • +4/-7     
    select.ts
    Remove acceptance notification and related code                   

    src/p2p/Join/v2/select.ts

  • Remove notifyNewestJoinedConsensors and related acceptance
    notification code.
  • Clean up unused notification functions.
  • +0/-79   
    shardus-types.ts
    Add config flag to server types for `/accepted` route       

    src/shardus/shardus-types.ts

  • Add enableAcceptedRoute to ServerConfiguration interface.
  • Document purpose of the new flag.
  • +2/-0     
    Documentation
    join-protocol-v2.md
    Update docs to reflect `/accepted` route deprecation         

    docs/join-protocol-v2/join-protocol-v2.md

  • Update join protocol documentation to reflect removal of /accepted
    endpoint.
  • Clarify new join verification flow using joined route.
  • +3/-2     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 93
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Feature Flag Gating

    The new enableAcceptedRoute flag is used to gate the deprecated /accepted route. Ensure that this gating works as intended and that the route is not accessible when the flag is false, to avoid unintended legacy behavior.

    external: [
      cycleMarkerRoute,
      joinRoute,
      joinedRoute,
      joinedV2Route,
      ...(config?.p2p?.enableAcceptedRoute ? [acceptedRoute] : []),
      unjoinRoute,
      standbyRefreshRoute,
    ],
    
    Removal of Acceptance Notification

    The notification logic for the deprecated /accepted endpoint has been removed. Confirm that all code paths and dependent features are updated accordingly, and that no references to the removed notification remain.

        for (const obj of objs) {
          if (obj.appJoinData?.adminCert?.goldenTicket === true && !selectedPublicKeys.has(obj.publicKey)) {
            /* prettier-ignore */ if (logFlags.p2pNonFatal && logFlags.console) console.log('selecting golden ticket nodes from standbyNodesInfo')
            selectedPublicKeys.add(obj.publicKey)
          }
        }
      }
    }
    /**
     * Returns the list of public keys of the nodes that have been selected and
     * empties the list.
     */
    

    Copy link

    PR Code Suggestions ✨

    No code suggestions found for the PR.

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

    Successfully merging this pull request may close these issues.

    1 participant