Skip to content

[ANCHOR-445] Remove unique_address callback#1446

Merged
philipliu merged 12 commits into
stellar:feature/v3from
philipliu:feature/anchor-445-remove-unique-address
Aug 26, 2024
Merged

[ANCHOR-445] Remove unique_address callback#1446
philipliu merged 12 commits into
stellar:feature/v3from
philipliu:feature/anchor-445-remove-unique-address

Conversation

@philipliu
Copy link
Copy Markdown
Contributor

@philipliu philipliu commented Aug 2, 2024

Description

This removes the unique_address call from the SEP-31 POST /transaction endpoint. stellar_account_id, stellar_memo, and stellar_memo_type are not required fields in the SEP-31 responses, so it's not necessary to generate the deposit address at transaction creation time. This PR updates the RequestOnchainFundsHandler RPC to allow businesses to populate the deposit address, similar to the SEP-6 and SEP-24 flows.

Context

We want to remove the unique_address callback in 3.0.

Testing

  • ./gradlew test

Documentation

SEP-31 integration guide and RPC docs will be updated.

Known limitations

N/A

@philipliu philipliu changed the base branch from develop to feature/v3 August 2, 2024 21:40
@philipliu philipliu force-pushed the feature/anchor-445-remove-unique-address branch from b7535d6 to 25ee2b1 Compare August 5, 2024 15:55
@philipliu philipliu force-pushed the feature/anchor-445-remove-unique-address branch 6 times, most recently from a45503c to e4b38f5 Compare August 20, 2024 15:51
@philipliu philipliu force-pushed the feature/anchor-445-remove-unique-address branch from e4b38f5 to 85aed2f Compare August 20, 2024 16:41
@philipliu philipliu marked this pull request as ready for review August 20, 2024 17:04
Comment thread core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java Outdated
Comment thread core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java
@@ -743,7 +737,7 @@ class Sep31ServiceTest {
val wantTx =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

memo in L735 is assigned twice.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

memo in L735 is assigned twice.

Nice catch, this variable is no longer used anymore. I deleted it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In https://github.com/stellar/java-stellar-anchor-sdk/pull/1446/files#diff-4e0835ca8055b8e40053d5f39ecebc0b44ac62e037db1367d0d3d639949a4dd3L4

Can we use a different package?

The link doesn't seem to work. Can you clarify what you mean?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah... I cannot recall. It is minor. May be will catch next time.

@philipliu philipliu force-pushed the feature/anchor-445-remove-unique-address branch from d9df1d7 to da0569d Compare August 21, 2024 17:28
Copy link
Copy Markdown
Contributor

@lijamie98 lijamie98 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -743,7 +737,7 @@ class Sep31ServiceTest {
val wantTx =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah... I cannot recall. It is minor. May be will catch next time.

@philipliu philipliu merged commit 75d8dc0 into stellar:feature/v3 Aug 26, 2024
@philipliu philipliu deleted the feature/anchor-445-remove-unique-address branch August 26, 2024 16:00
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