feat(autoclaim): identify API requests by global index

[arnaubennassar]

🔄 Changes Summary

• The autoclaim API now identifies requests by their claim global index (same encoding as the bridge API) instead of the custom source_network:destination_network:deposit_count ID.
• GET /autoclaim/v1/bridges/{global_index}, POST .../{global_index}/approve and .../reject take a decimal or 0x-prefixed hex global index. Non-numeric values return 400.
• GET /autoclaim/v1/bridges gains a global_index filter.
• The id field is removed from the request response (global_index was already present).
• New Storage.GetRequestByGlobalIndex; the internal request key is unchanged (no migration).
• Swagger regenerated, docs/autoclaim.md / docs/e2e_tests.md updated, e2e helpers convert keys to global indexes.

⚠️ Breaking Changes

• 🔌 API/CLI: autoclaim {id} path parameter replaced by {global_index}; id response field removed.

🔌 API Updates

🔌 Others API

• 🔌 Autoclaim API Update: modified GET /autoclaim/v1/bridges/{id} → {global_index}, POST /autoclaim/v1/bridges/{id}/approve|reject → {global_index}; added global_index query filter on GET /autoclaim/v1/bridges; removed id from responses. Breaking interface change.

✅ Testing

• 🤖 Automatic: new/updated unit tests in autoclaim/api, autoclaim/apitypes, autoclaim/storage (lookup, hex input, invalid input, not found, ambiguous, list filter); e2e helpers updated (not run locally). golangci-lint clean on ./autoclaim/... ./test/e2e/....

🐞 Issues

• Closes change format of autoclaim api to be consistent with global index used in bridge api #1881

📝 Notes

• The global index does not encode the destination network. In the rare case a legacy pre-Etrog bare deposit-count index equals a rollup-origin one, the by-index routes return 409; the list endpoint with global_index + destination_network can disambiguate, but approve/reject cannot target such a request.
• No DB index was added on global_index; lookups scan autoclaim_request. Can add a migration if the table is expected to be large.

🤖 Generated with Claude Code

https://claude.ai/code/session_017eyBoQ7G2hoShKEzAQvZWM

[arnaubennassar]

@claude

[claude]

Claude finished @arnaubennassar's task in 51s —— View job

---

Review of PR #1891: identify autoclaim API requests by global index

• Read the PR diff and the surrounding storage, API, docs and e2e code
• Check consistency with the bridge API's global-index parsing
• Write the review

Overall this is a clean, well-scoped change. The internal request key is untouched, so no migration is needed. Error mapping (400/404/409) is consistent across the public and admin routes, and the unit tests cover decimal input, hex input, invalid input, not found, ambiguous and the list filter. I found no 🔴 blockers. I only read the code and did not build or run the tests, and the e2e changes were not run either.

🟡 Suggested

1. Missing index on global_index makes every lookup two full scans (autoclaim/storage/storage.go:626-650, listRequests at :1079).

   • GetRequestByGlobalIndex goes through ListRequests, which runs SELECT COUNT(*) and then the SELECT … LIMIT 2. Both filter on global_index = ?, which has no index.
   • The by-index GET and the approve/reject routes are now the primary access path, and the public GET is exposed on the bridge service. Autoclaim tables can grow large on busy networks.
   • A small autoclaim0004.sql with CREATE INDEX … ON autoclaim_request(global_index) would remove this cost.
   • Alternatively, query directly with LIMIT 2 and skip the count. The count is only needed to detect ambiguity, and fetching 2 rows already tells you that.

2. Ambiguous global indexes cannot be approved or rejected (autoclaim/api/admin.go:204-215, the ErrAmbiguousGlobalIndex case in writeStorageError).

   • You documented this limitation, but it is a dead end for the operator. A request in manual-approval-required that collides with another request is permanently unactionable through the API.
   • Consider accepting an optional destination_network query parameter on GET/approve/reject (reusing the filter already added for the list endpoint). That makes every request addressable, and the cost is one extra filter field.
   • If you would rather not do that now, a follow-up issue would be worth opening.

3. ParseGlobalIndex uses base 0, so a leading zero silently changes the value (autoclaim/apitypes/filter.go:83).

   • SetString(v, 0) treats 010 as octal 8, and also accepts 0b…, 0o… and _ separators (for example 1_000).
   • This matches bridgeservice/utils.go:90, so the behaviour is consistent with the bridge API. The "decimal or 0x-prefixed hex" docs and swagger text slightly overpromise, though.
   • Option: either document it as "same parsing as the bridge API", or restrict input to decimal and 0x hex. If you restrict it, do it in both places, or the two APIs will diverge.
   • Related nit: this helper adds TrimSpace and rejects negatives, but the bridge API does neither. Harmless, but not literally "same encoding".

🟢 Optional

• The ErrorResponse example (autoclaim/apitypes/types.go:16) reads request with global index … not found. The actual not-found error is get autoclaim request by global index X: not found. Align the example, or the error text, so the swagger doc is accurate.
• GetRequestByGlobalIndex has the check page.Count == 0 || len(page.Requests) == 0. The second condition only hides an inconsistency between the count query and the row query. It is fine as defensive code, but it can never mean "not found" if Count > 0.
• autoClaimGlobalIndex in test/e2e/autoclaim_test.go re-derives the global index from the key. That is correct for the e2e cases, which use post-Etrog indexes. If the stored index ever came from a pre-Etrog bridge event (bridgesync.GlobalIndexForBridge), the derived value would differ from the stored one. Reading global_index from the list response would be more robust, but that is not needed now.
• docs/autoclaim.md: the new global_index line in the list-parameters paragraph exceeds 120 columns. This is not enforced for markdown, but it is inconsistent with the surrounding wrapping.

Security and performance notes

• No new injection surface: the filter value is passed as a bound parameter (global_index = ?) after being parsed to a big.Int.
• Unbounded-size input: a very large decimal or hex string is parsed into a big.Int before any bound check. Gin and the HTTP server limit the URL length, so this is low risk, but the same applies to the bridge API.
• Breaking change: the {id} to {global_index} change and the removal of id from responses will break existing clients with no deprecation window. That is explicitly called out in the PR, so just make sure it lands in the release notes.

---

🚀 What's New

• The autoclaim API now identifies requests by their claim global index, using the same encoding as the bridge API (decimal, or 0x-prefixed hex), instead of the custom source_network:destination_network:deposit_count ID.
• GET /autoclaim/v1/bridges/{global_index} and POST /autoclaim/v1/bridges/{global_index}/approve|reject replace the {id} routes.
• GET /autoclaim/v1/bridges gains a global_index filter.
• New Storage.GetRequestByGlobalIndex, which returns 404 if nothing matches and 409 if more than one request shares the index. The internal request key is unchanged.
• Swagger, docs/autoclaim.md, docs/e2e_tests.md and the e2e helpers are updated.

🐛 Bug Fixes

None.

📋 Config Updates

None.

⚠️ Breaking Changes

• The autoclaim {id} path parameter is replaced by {global_index} on the get, approve and reject routes.
• The id field is removed from the request response (global_index was already present).
• Non-numeric {global_index} values return 400.
• Existing clients using source:destination:deposit_count IDs must switch to global indexes.

---

· branch feat/1881-autoclaim-api-global-index

[claude]

Claude finished @joanestebanr's task in 27s —— View job

---

Review of PR #1891: identify autoclaim API requests by global index

This is a clean, well-scoped change. I found no 🔴 blockers. I read the code and did not build or run the tests, and the e2e changes were not run.

• Matching: global_index is stored as decimal TEXT (autoclaim0001.sql:18, written via request.GlobalIndex.String() at storage.go:1251). The filter compares against GlobalIndex.String() (storage.go:1176), so hex input is normalized to decimal before the lookup and matches correctly.
• No NULLs: storage.go:507 derives a global index when none is set, so rows should never have a NULL one.
• Errors and SQL: error mapping is consistent on the public and admin routes (400, 404, 409). The filter value is a bound parameter, so there is no injection surface.
• Migration: the internal request key is unchanged, so none is needed.

🟡 Suggested

1. No index on global_index, and each lookup runs two scans (autoclaim/storage/storage.go:626-650).

   • GetRequestByGlobalIndex goes through ListRequests, which runs COUNT(*) and then a LIMIT 2 select. Both filter on the unindexed global_index.
   • These lookups are now the main access path (public GET, approve, reject), so a small autoclaim0004.sql with CREATE INDEX … ON autoclaim_request(global_index) would help.
   • You could also query directly with LIMIT 2 and skip the count, since two rows already signal ambiguity.

2. Ambiguous global indexes cannot be approved or rejected (autoclaim/api/admin.go:290-298).

   • The 409 is documented, but a collided request in manual-approval-required can never be actioned through the API.
   • Consider an optional destination_network query parameter on GET, approve and reject. The filter field already exists. If you would rather not do that here, open a follow-up issue.

3. ParseGlobalIndex uses base 0 (autoclaim/apitypes/filter.go:83).

   • SetString(v, 0) reads 010 as octal 8 and also accepts 0b…, 0o… and _ separators.
   • This matches bridgeservice/utils.go:90, so it is consistent with the bridge API. The "decimal or 0x-prefixed hex" text in swagger and docs slightly overpromises.
   • Either document it as "same parsing as the bridge API", or restrict input in both places so the two APIs do not diverge.
   • This helper also trims whitespace and rejects negatives, which the bridge API does not.

🟢 Optional

• Swagger example mismatch: the ErrorResponse example (autoclaim/apitypes/types.go:16) says request with global index … not found. The real error is get autoclaim request by global index X: not found.
• Redundant check: page.Count == 0 || len(page.Requests) == 0 (storage.go:643) is defensive only. Count > 0 with no rows would be an inconsistency, not "not found".
• E2E robustness: autoClaimGlobalIndex in test/e2e/autoclaim_test.go re-derives the index from the key. That is fine for post-Etrog cases, but it would differ from the stored value for a pre-Etrog index. Reading global_index from the list response is more robust.
• Docs formatting: the new global_index line in docs/autoclaim.md exceeds 120 columns, unlike the surrounding wrapping.

Security and performance notes

• A very large decimal or hex string is parsed into a big.Int before any bound check. URL length limits make this low risk, and the bridge API behaves the same way.
• The {id} to {global_index} change and the removal of id from responses break existing clients with no deprecation window. The PR calls this out, so make sure it lands in the release notes.

---

🚀 What's New

• The autoclaim API now identifies requests by claim global index, using the bridge API's parsing (decimal or 0x hex), instead of source:destination:deposit_count IDs.
• GET /autoclaim/v1/bridges/{global_index} and POST /autoclaim/v1/bridges/{global_index}/approve|reject replace the {id} routes.
• GET /autoclaim/v1/bridges gains a global_index filter.
• New Storage.GetRequestByGlobalIndex returns 404 if nothing matches and 409 if several requests share the index. The internal request key is unchanged, so no DB migration.
• Swagger, docs/autoclaim.md, docs/e2e_tests.md and the e2e helpers are updated.

🐛 Bug Fixes

None.

📋 Config Updates

None.

⚠️ Breaking Changes

• The autoclaim {id} path parameter is replaced by {global_index} on the get, approve and reject routes.
• The id field is removed from the request response (global_index was already present).
• Non-numeric {global_index} values return 400.
• Existing clients using source:destination:deposit_count IDs must switch to global indexes.

--- · branch feat/1881-autoclaim-api-global-index

[arnaubennassar]

Addressed the review suggestions in the latest commit:

• Double scan: GetRequestByGlobalIndex now does one LIMIT 2 query (no COUNT). I did not add a global_index DB index/migration; happy to add one if you want it.
• Ambiguous index dead end: GET /bridges/{global_index}, approve and reject accept an optional destination_network query param, so colliding requests can be targeted. Tests and docs updated.
• Base-0 parsing: docs/swagger now say it is parsed like the bridge API (kept identical on purpose, plus space trimming and negative rejection).
• Nits: swagger error example matches the real error, redundant not-found check removed, long docs line wrapped.
• Not changed: the e2e helper still derives the index from the key (e2e only uses post-Etrog indexes).

🤖 Generated with Claude Code

https://claude.ai/code/session_017eyBoQ7G2hoShKEzAQvZWM