Skip to content
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

[ANCHOR-812] Deprecate on_chagne_callback of SEP-6 and SEP-24 in favor of STATUS_CALLBACK_URL of SEP-1 #1600

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lijamie98
Copy link
Contributor

@lijamie98 lijamie98 commented Jan 3, 2025

Currently, in AP we configure wallet callback in the config and don’t support passing it as a request parameter. However, in the SEP that’s still the case. We should discuss the strategy, and potentially deprecate/remove callback argument from request parameters in favor of communicating URL between Anchor and Wallet (potentially utilizing SEP-1)

In the withdraw and deposit requests, the wallet may optionally provide the URL for the anchor to call back when the transaction status is changed. However, this may pose a potential threat when requesting the anchor's server to call an arbitrary server externally. This issue was raised by one of the anchors requesting AP not to allow the on_change_callback from the withdraw/deposit requests but instead add the wallet callback URL in the configuration.

In this PR, the SEP-6/on_change_callback and SEP-24/on_change_callback are deprecated. The SEP-1 is amended to specify the wallet's callback URLs in the General Information of the TOML in wallet's stellar.toml file.

Slack Discussion: https://stellarfoundation.slack.com/archives/C036T11AY84/p1733509062701459

@lijamie98 lijamie98 changed the title Deprecate on_chagne_callback of SEP-6 and SEP-24 in favor of STATUS_CALLBACK_URL of SEP-1 [ANCHOR-812] Deprecate on_chagne_callback of SEP-6 and SEP-24 in favor of STATUS_CALLBACK_URL of SEP-1 Jan 3, 2025
@@ -74,6 +74,7 @@ These are global fields in the `stellar.toml` file.
| URI_REQUEST_SIGNING_KEY | Stellar public key | The signing key is used for [SEP-7](sep-0007.md) delegated signing |
| DIRECT_PAYMENT_SERVER | uses `https://` | The server used for receiving [SEP-31](sep-0031.md) direct fiat-to-fiat payments. Requires [SEP-12](sep-0012.md) and hence a `KYC_SERVER` TOML attribute. |
| ANCHOR_QUOTE_SERVER | uses `https://` | The server used for receiving [SEP-38](sep-0038.md) requests. |
| STATUS_CALLBACK_URL | uses `https://` | The URL used for receiving [SEP-6](sep-0006.md) and [SEP-24](sep-0024.md) transaction status change callbacks. |
Copy link
Contributor

Choose a reason for hiding this comment

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

SEP-31 and SEP-12 also have callbacks. The schema for SEP-12 looks like the customer object instead of a transaction like the rest of the SEPs. Anchor Platform allows configuring each SEP specific callback URLs. Should we consider allowing that here?


The wallet server can optionally provide the Anchor with a URL to send a callback to when the status of a transaction
has changed via the `STATUS_CALLBACK_URL` field in the wallet's [`stellar.toml`](sep-0001.md). The Anchor should POST a
JSON message to when the status property of the transaction created as a result of this request changes. The JSON
Copy link
Contributor

Choose a reason for hiding this comment

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

The Anchor should POST a JSON message to when the status property of the transaction created as a result of this request changes.

Not sure I understand this sentence. What does "this request" refer to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased to avoid the confusion.

message should be identical to the response format for the /transaction endpoint. The callback needs to be signed by the
anchor and the signature needs to be verified by the wallet according to the callback signature specification.

The payload of the callback should be the same as the response of the `GET /transaction` endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is repeated in the paragraph above.

| `memo` | string | (optional) Value of memo to attach to transaction, for `hash` this should be base64-encoded. Because a memo can be specified in the SEP-10 JWT for [Shared Accounts](#shared-omnibus-or-pooled-accounts), this field as well as `memo_type` can be different than the values included in the SEP-10 JWT. For example, a client application could use the value passed for this parameter as a reference number used to match payments made to `account`. |
| `email_address` | string | (optional) Email address of depositor. If desired, an anchor can use this to send email updates to the user about the deposit. |
| `lang` | string | (optional) Defaults to `en` if not specified or if the specified language is not supported. Language code specified using [RFC 4646]. `error` fields and other human readable messages in the response should be in this language. |
| `on_change_callback` | string | (**deprecated** in favor of `STATUS_CALLBACK_URL` of the [SEP-1](sep0001.md)). |
Copy link
Contributor

Choose a reason for hiding this comment

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

For other deprecated fields, we leave the description. Do we want to do that here?

@@ -74,6 +74,7 @@ These are global fields in the `stellar.toml` file.
| URI_REQUEST_SIGNING_KEY | Stellar public key | The signing key is used for [SEP-7](sep-0007.md) delegated signing |
| DIRECT_PAYMENT_SERVER | uses `https://` | The server used for receiving [SEP-31](sep-0031.md) direct fiat-to-fiat payments. Requires [SEP-12](sep-0012.md) and hence a `KYC_SERVER` TOML attribute. |
| ANCHOR_QUOTE_SERVER | uses `https://` | The server used for receiving [SEP-38](sep-0038.md) requests. |
| STATUS_CALLBACK_URL | uses `https://` | The URL used for receiving [SEP-6](sep-0006.md) and [SEP-24](sep-0024.md) transaction status change callbacks. |
Copy link
Contributor

Choose a reason for hiding this comment

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

We should specify that including this field in the stellar.toml does not guarantee that Anchors will send callback events to the wallet server. Anchors are not required to support callbacks and some of them only send callbacks to URLs that are whitelisted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes the ### On Change Callback URL section of the related SEPs.


The payload of the callback should be the same as the response of the `GET /transaction` endpoint.
has changed via the `STATUS_CALLBACK_URL` field in the wallet's [`stellar.toml`](sep-0001.md). Depending on the policy,
the Anchor may POST a JSON message to the callback URL when the status property of the transaction changes. The JSON
Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on the policy, the Anchor may POST a JSON message to the callback URL when the status property of the transaction changes.

It's not clear what the policy refers to, so I think we can remove it. We can rephrase it to something like "The Anchor should POST a JSON message to the callback URL when the transaction's status changes`.

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