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-704] Clean up amount_fee and add user_action_required_by #935

Merged
merged 7 commits into from
Aug 28, 2024

Conversation

JiahuiWho
Copy link
Contributor

As title

@JiahuiWho
Copy link
Contributor Author

This will go to 3.0 release as 2.9 has already been frozen. Version doc release is controversial to code release, no major change should be stack after release has been cut

@stellar-jenkins
Copy link

@@ -1064,8 +1090,8 @@ components:
$ref: '#/components/schemas/Amount'
amount_out:
$ref: '#/components/schemas/Amount'
amount_fee:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove this field? I know it's deprecated in 2.9 and we removed deprecated fields from the docs in the past but now that we have versioned docs, I think we should keep it. WDYT?

Copy link
Contributor Author

@JiahuiWho JiahuiWho Aug 22, 2024

Choose a reason for hiding this comment

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

We always remove deprecated fields from docs. This amount_fee is totally removed from platform transaction in 3.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should not remove deprecated fields now that we have versioning but I'm okay with merging as is.

Copy link
Contributor Author

@JiahuiWho JiahuiWho Aug 22, 2024

Choose a reason for hiding this comment

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

I meant to wait till after the 2.9 release to merge it. In this way it will goes to 3.0

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I think next should always be 3.0
We can copy existing docs for 2.9 to make docs for 2.10 (if necessary)

@stellar-jenkins
Copy link

@@ -1064,8 +1090,8 @@ components:
$ref: '#/components/schemas/Amount'
amount_out:
$ref: '#/components/schemas/Amount'
amount_fee:
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I think next should always be 3.0
We can copy existing docs for 2.9 to make docs for 2.10 (if necessary)

@@ -109,6 +113,8 @@ SECRET_SEP24_MORE_INFO_URL_JWT_SECRET="your encryption key shared with your busi

`SECRET_SEP24_INTERACTIVE_URL_JWT_SECRET` and `SECRET_SEP24_MORE_INFO_URL_JWT_SECRET` are encryption keys that the Anchor Platform will use to generate short-lived tokens it will add to the URLs provided to the wallet. Your business server must also have these keys in its environment so it can verify the token's signature.

`SEP24_INITIAL_USER_DEADLINE_SECONDS` is an optional param that defines the time in seconds a user has to act before the transaction moves to the next status. It determines the `user_action_required_by` field, which indicates the deadline. Check [JSON-RPC Methods][json-rpc-methods] for usage examples.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind adding a paragraph of how it's supposed to be used?
(i.e. business should call get /transaction and check with user_action_required_by has passed, and manually execute the logic. For example, they can expire the transaction, or auto-refund, depending on the tx status)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do want to keep this part focused on all the configuration and do not stretch too much.

The use of user_action_required_by should go to where's really being applied, e.g RPC method section, or platform transaction

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm that's fair. Maybe we should include it in SEP-24/SEP-6 guide instead? This section makes sense to include this information in.

@stellar-jenkins
Copy link

@JiahuiWho JiahuiWho requested a review from Ifropc August 27, 2024 15:17
@stellar-jenkins
Copy link

@stellar-jenkins
Copy link

@JiahuiWho JiahuiWho merged commit 1f9e290 into main Aug 28, 2024
1 of 2 checks passed
@JiahuiWho JiahuiWho deleted the anchor-704-3.0 branch August 28, 2024 13:40
@stellar-jenkins
Copy link

janewang pushed a commit that referenced this pull request Sep 12, 2024
…935)

* amount_fee and user_action_required_by

* Update platforms/anchor-platform/admin-guide/sep6/configuration.mdx

Co-authored-by: Philip Liu <[email protected]>

* add user_action_required_by to GET /transactions query param

* add instruction in sep6 and sep24

* Update platforms/anchor-platform/admin-guide/sep24/integration.mdx

Co-authored-by: Gleb <[email protected]>

* prettier

---------

Co-authored-by: Philip Liu <[email protected]>
Co-authored-by: Gleb <[email protected]>
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.

4 participants