Skip to content

Remove tx/create-sponsored-account endpoint #267

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: graphql-main
Choose a base branch
from

Conversation

akcays
Copy link

@akcays akcays commented Aug 7, 2025

What

This PR removes:

  1. HTTP Handler and Route:
    - Removes SponsorAccountCreation method from AccountHandler
    - Removes route definition
    - Removes SponsorAccountCreationRequest struct
  2. Account Sponsorship Service:
    - Removes SponsorAccountCreationTransaction method from the interface and implementation
    - Removes unused constants: ErrAccountAlreadyExists, ErrSponsorshipLimitExceeded, CreateAccountTxnTimeBounds, CreateAccountTxnTimeBoundsSafetyMargin
  3. Tests:
    - Removes TestAccountHandlerSponsorAccountCreation from account_handler_test.go
    - Removes TestAccountSponsorshipServiceSponsorAccountCreationTransaction from account_sponsorship_service_test.go
    - Updates remaining test to use simplified service options
    - Removes method from AccountSponsorshipServiceMock
  4. Unused Imports and Dependencies:
    - Clean up imports
    - Removes unused variables and struct fields

Why

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

Known limitations

N/A

Issue that this PR addresses

#233

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.

@akcays akcays force-pushed the remove-tx/create-sponsored-account branch from f2a025a to a413f8f Compare August 8, 2025 17:53
@akcays akcays marked this pull request as ready for review August 11, 2025 18:10
@akcays akcays force-pushed the remove-tx/create-sponsored-account branch from a413f8f to 830dd18 Compare August 11, 2025 20:03
Copy link
Contributor

@aditya1702 aditya1702 left a comment

Choose a reason for hiding this comment

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

LGTM. Please remove the reference from openapi yaml before merging.


return txe, s.ChannelAccountSignatureClient.NetworkPassphrase(), nil
}

// WrapTransaction wraps a stellar transaction with a fee bump transaction with the configured distribution account as the fee account.
func (s *accountSponsorshipService) WrapTransaction(ctx context.Context, tx *txnbuild.Transaction) (string, string, error) {
Copy link
Contributor

@aditya1702 aditya1702 Aug 12, 2025

Choose a reason for hiding this comment

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

I think we should move WrapTransaction from here since its only used for creating fee-bump txns. Will leave it up to you to do it in this PR or the create-fee-bump one, and then completely delete this file

Copy link
Author

Choose a reason for hiding this comment

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

Good callout! I agree that it would be ideal to move WrapTransaction, and remove account_sponsorship_service.

I was planning on moving it into it's own service (i.e. feeBumpService), with create-fee-bump but doing it in this PR will be cleaner/easier to review as create-fee-bump will also include the migration logic.

@@ -8,63 +8,16 @@ import (
"github.com/stellar/go/txnbuild"
Copy link
Contributor

Choose a reason for hiding this comment

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

You should also remove the endpoint from the openapi/main.yaml file

@akcays akcays force-pushed the remove-tx/create-sponsored-account branch from 830dd18 to 883d48e Compare August 12, 2025 18:08
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