Skip to content

Conversation

aditya1702
Copy link
Contributor

What

[TODO: Short statement about what is changing.]

Why

[TODO: Why this change is being made. Include any context required to understand the why.]

Known limitations

[TODO or N/A]

Issue that this PR addresses

[TODO: Attach the link to the GitHub issue or task. Include the priority of the task here in addition to the link.]

Checklist

PR Structure

  • It is not possible to break this PR down into smaller PRs.
  • This PR does not mix refactoring changes with feature changes.
  • This PR's title starts with name of package that is most changed in the PR, or all if the changes are broad or impact many packages.

Thoroughness

  • This PR adds tests for the new functionality or fixes.
  • All updated queries have been tested (refer to this check if the data set returned by the updated query is expected to be same as the original one).

Release

  • This is not a breaking change.
  • This is ready to be tested in development.
  • The new functionality is gated with a feature flag if this is not ready for production.

aditya1702 and others added 19 commits July 21, 2025 10:27
* Add registerAccount and deregisterAccount mutations

* Remove the REST endpoints for registerAccount and deregisterAccount

* Add tests for registerAccount and deregisterAccount mutations

* RegisterAccount mutation should return error on conflict

* Adjust existing tests to support the new behavior

* Complete REST to GraphQL migration for /accounts/ endpoints

* Remove ON CONFLICT DO NOTHING from account insertion to detect duplicates
* Add custom errors: ErrAccountAlreadyExists and ErrAccountNotFound
* Update Insert method to detect duplicate account errors
  using PostgreSQL error codes
* Update Delete method to check RowsAffected() and return
  ErrAccountNotFound for non-existent accounts
* Add tests for duplicate registration and non-existent account
  deregistration
* Implement GraphQL error codes:
  - ACCOUNT_ALREADY_EXISTS for duplicate registrations
  - ACCOUNT_NOT_FOUND for deregistering non-existent accounts
  - ACCOUNT_REGISTRATION_FAILED for registration failures
  - ACCOUNT_DEREGISTRATION_FAILED for deregistration failures
* Update resolver tests to expect GraphQL errors

REST Cleanup:
* Remove RegisterAccount and DeregisterAccount methods
* Remove related REST handler tests for the removed routes

* Update assertions to use assert.ErrorContains

* Add address validation

* Remove deadcode

* Add success to registerAccountPayload for consistency

* Rename isValidStellarAddress method to isValidAddress

* Update assertion

* Update the method to return wrapped error

* Fix goimports errors
* Move /transactions/build endpoint from REST to GraphQL

* Fix shadowing error

* Add tests for soroban smart contract simulation

* Fix imports check

* Fix test error

* Fix build transaction integration tests

* Fix test

* Fix lint check

* Remove transactions http handler

* Update type names to transaction

* Update tests

* Add custom errors to handle different tx validation failures

Transaction Validation:
 * ErrInvalidTimeout - Timeout exceeds maximum allowed
 * ErrInvalidOperationChannelAccount - Operation uses channel account as source
 * ErrInvalidOperationMissingSource - Non-Soroban operation missing source account

Soroban-Specific Validation:
 * ErrInvalidSorobanOperationCount - Must have exactly one operation
 * ErrInvalidSorobanSimulationEmpty - Missing simulation response
 * ErrInvalidSorobanSimulationFailed - Simulation failed with error
 * ErrInvalidSorobanOperationType - Unsupported operation type

* Remove deadcode

* Fix goimports check

* Fix tests

* Update errors add logs and todos

* Update tests

* Remove redundant error handling

* Refactor
* Remove /tx/create-sponsored-account endpoint

* Remove accountSponsorhipService and move wrapTransaction to feeBumpService
* update docker-compose.yaml

* Remove audience claim from JWT authentication

Removes the `audience` claim from JWTs. This claim did not meaningfully improve security for wallet backend instances, and while it did ensure JWTs could not be used for instances of the wallet backend running on different hosts, this is a defensive measure for the client's sake, not the server's. However, this does simplify client implementations and reduce the chances of misconfiguration. For example, when I run the wallet backend using docker-compose, the URL I make requests with have `localhost` hostnames, but the hostname of the server is actually `api` as its defined within the compose file. This possibility, for the domain clients must send requests to differ from the hostname configured for the instance, forces client SDKs to have one parameter for the URL to make requests to and a separate parameter for the `audience`.

## Changes Made:

### JWT Interface Updates:
- Removed `audience` parameter from `JWTTokenParser.ParseJWT()` and `JWTTokenGenerator.GenerateJWT()` interfaces
- Updated `HTTPRequestVerifier.VerifyHTTPRequest()` to no longer require audience parameter

### Core Logic Updates:
- Removed audience validation logic from `claims.Validate()` function
- Removed unused `slices` import from claims.go
- Removed `Audience` field generation from JWT tokens
- Removed hostname parsing logic that was only used for audience validation

### Middleware Updates:
- Updated `AuthenticationMiddleware()` to no longer require `serverHostname` parameter
- Removed `ServerHostname` field from `handlerDeps` struct since it was only used for audience validation
- Removed unused `net/url` import from serve.go

### Test Cleanup:
- Removed audience-only test cases: `🔴invalid_audience` and `🔴invalid_hostname`
- Updated all remaining tests to remove audience parameters and references
- Removed unused variables like `validAudience`, `invalidAudience`, `validHostname`, `invalidHostname`

### Documentation Update:
- Removed `aud` claim documentation from README.md

## What's Still Preserved:

- **`SERVER_BASE_URL` configuration** - Still used by PaymentService for building pagination URLs
- **All other JWT validations** - Subject, method/path, body hash, expiration, etc. remain intact
- **All other security mechanisms** - JWT signature validation, token expiration, etc. still work

## Verification:

- ✅ All tests pass in auth and middleware packages
- ✅ Entire codebase compiles successfully
- ✅ No remaining audience/aud references found
- ✅ Minimal impact achieved - only audience-specific functionality removed

The changes are surgical and focused solely on removing audience claim validation while preserving all other authentication mechanisms and functionality.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

* Fix formatting issues - remove extra blank lines

Applied golangci-lint --fix to resolve CI formatting failures

* move back to stable rpc image

---------

Co-authored-by: Claude <[email protected]>
* Remove /payments/ endpoint, models, and ingestion logic

* Code review changes
* make JWT auth optional

* update custom env var parser

* remove start-ledger

* update custom value setter test to expect passing on empty list
Copy link

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