feat(backend): add deactivateServiceOnChain and POST /api/services/:id/deactivate route#192
Open
leocagli wants to merge 1 commit into
Open
Conversation
…d/deactivate Closes Stellar-Ecosystem#14 The Soroban registry contract exposes deactivate_service(provider, id) with provider.require_auth() and provider == entry.provider authorization, but no backend route exposed it — providers using the server keypair had no API path to deactivate their listings after registration. Added: - deactivateServiceOnChain(id) in backend/src/lib/contract.js: builds and submits the deactivate_service call signed by the server keypair (which is the provider for services registered via registerServiceOnChain / seed scripts) - POST /api/services/:id/deactivate in backend/src/routes/registry.js: validates the ID, checks the service exists and is active, then calls deactivateServiceOnChain; returns 404 if not found, 409 if already inactive, 500 on chain error
📝 WalkthroughWalkthroughA new ChangesService Deactivation Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/routes/registry.js`:
- Around line 153-156: The current code at Line 154 returns a 404 status for any
falsy value from getService(id), but getService() returns null for both missing
services and contract read errors. Modify the logic to distinguish between these
two cases: either update getService() to return a distinct value or error object
for read failures, or enhance the route handler to catch exceptions from
getService(id) separately and map those failures to a 5xx status code, while
reserving 404 only for cases where the service genuinely does not exist.
- Around line 145-163: The deactivate service route handler currently only
applies writeRateLimiter() middleware, allowing any unauthenticated user to
deactivate services. Add authentication middleware before or after the
writeRateLimiter() call in the router.post("/services/:id/deactivate", ...)
handler to verify the caller's identity. Additionally, after retrieving the
service with getService(id), add authorization logic to verify that the
authenticated caller is the provider/owner of that service before allowing the
deactivateServiceOnChain(id) operation to proceed. Return a 403 Forbidden status
if the caller is not authorized to deactivate that particular service.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fd6f4284-032c-4c56-925d-b4557fd5f01d
📒 Files selected for processing (2)
backend/src/lib/contract.jsbackend/src/routes/registry.js
Comment on lines
+145
to
+163
| router.post("/services/:id/deactivate", writeRateLimiter(), async (req, res) => { | ||
| let id; | ||
| try { | ||
| id = parseInt(req.params.id, 10); | ||
| if (isNaN(id) || id < 1) { | ||
| return res.status(400).json({ error: "Invalid service ID", code: "INVALID_ID" }); | ||
| } | ||
|
|
||
| const service = await getService(id); | ||
| if (!service) { | ||
| return res.status(404).json({ error: "Service not found", code: "NOT_FOUND" }); | ||
| } | ||
| if (!service.active) { | ||
| return res.status(409).json({ error: "Service is already deactivated", code: "ALREADY_DEACTIVATED" }); | ||
| } | ||
|
|
||
| await deactivateServiceOnChain(id); | ||
| logger.info({ id }, "Service deactivated"); | ||
| res.json({ success: true, id }); |
There was a problem hiding this comment.
Add provider authentication/authorization to this write route.
At Line 145, only writeRateLimiter() is enforced. This allows unauthenticated callers to invoke deactivation, and because the backend signs with the server keypair, callers can deactivate server-owned listings without proving provider identity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/routes/registry.js` around lines 145 - 163, The deactivate
service route handler currently only applies writeRateLimiter() middleware,
allowing any unauthenticated user to deactivate services. Add authentication
middleware before or after the writeRateLimiter() call in the
router.post("/services/:id/deactivate", ...) handler to verify the caller's
identity. Additionally, after retrieving the service with getService(id), add
authorization logic to verify that the authenticated caller is the
provider/owner of that service before allowing the deactivateServiceOnChain(id)
operation to proceed. Return a 403 Forbidden status if the caller is not
authorized to deactivate that particular service.
Comment on lines
+153
to
+156
| const service = await getService(id); | ||
| if (!service) { | ||
| return res.status(404).json({ error: "Service not found", code: "NOT_FOUND" }); | ||
| } |
There was a problem hiding this comment.
Don’t return NOT_FOUND for upstream read failures.
At Line 153, getService(id) is used as an existence check, but backend/src/lib/contract.js returns null on read exceptions too. That makes Line 154 emit 404 during RPC/contract failures, masking outages as missing services. Please split “service not found” from “contract read failed” and map read failures to 5xx.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/routes/registry.js` around lines 153 - 156, The current code at
Line 154 returns a 404 status for any falsy value from getService(id), but
getService() returns null for both missing services and contract read errors.
Modify the logic to distinguish between these two cases: either update
getService() to return a distinct value or error object for read failures, or
enhance the route handler to catch exceptions from getService(id) separately and
map those failures to a 5xx status code, while reserving 404 only for cases
where the service genuinely does not exist.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14
Problem
The Soroban registry contract exposes
deactivate_service(provider, id)withprovider.require_auth()authorization checks, but no backend route called it. Providers using the server keypair (services registered viaregisterServiceOnChain/ seed scripts /update-endpoints.js) had no API path to deactivate listings — only direct Soroban CLI invocation worked.Changes
backend/src/lib/contract.js— newdeactivateServiceOnChain(id):backend/src/routes/registry.js— newPOST /api/services/:id/deactivate:INVALID_IDNOT_FOUNDALREADY_DEACTIVATEDDEACTIVATE_ERROR{ success: true, id }The route is rate-limited via
writeRateLimiter()to match the other write endpoints.🤖 Generated with Claude Code
Summary by CodeRabbit