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-775] Implement SEP-45 #1623

Merged

Conversation

philipliu
Copy link
Contributor

@philipliu philipliu commented Jan 22, 2025

Description

This PR implements the core logic of SEP-45.

SEP-45 shares a common configuration with SEP-10 so I decided to keep the SEP-45 config simple and fix it at a later point in time. Tests and comments for the configuration have not been implemented as part of this PR.

Context

C-account implementation

Testing

  • ./gradlew test

Documentation

Stellar docs need to be updated

Known limitations

Client domain verification has not been tested yet.

@philipliu philipliu changed the base branch from develop to feature/c-accounts January 22, 2025 21:53
@philipliu philipliu force-pushed the feature/anchor-755-sep-45 branch 3 times, most recently from 32ffa58 to 4c2f3b2 Compare January 23, 2025 01:31
@philipliu philipliu force-pushed the feature/anchor-755-sep-45 branch from 4c2f3b2 to 48d9e11 Compare January 23, 2025 02:41
@philipliu philipliu force-pushed the feature/anchor-755-sep-45 branch from 9532d94 to 43a9752 Compare January 23, 2025 04:15
@philipliu philipliu changed the title [ANCHOR-755] Implement SEP-45 [ANCHOR-775] Implement SEP-45 Jan 23, 2025
@philipliu philipliu marked this pull request as ready for review January 23, 2025 04:42
@@ -70,6 +73,18 @@ void validateConfig(AppConfig config, Errors errors) {
config.getHorizonUrl()));
}
}

if (isEmpty(config.getRpcUrl())) {
// TODO: this can only be empty when SEP-45 is disabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe check this in sep45Config like checking secretConfig there?

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 created ANCHOR-955 to add all the validation + refactor the configuration a little bit. This PR is the bare minimum to get the implementation working.

public class PropertySep45Config implements Sep45Config, Validator {
private Boolean enabled;
private String webAuthDomain;
private String webAuthContractId;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not. The configs will be validated in ANCHOR-955.

/**
* Creates the arguments for the web_auth_verify function.
*
* @param request the challenge request
Copy link
Contributor

Choose a reason for hiding this comment

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

comment

@JiahuiWho
Copy link
Contributor

LGTM. Feel free to merge once the minors are addressed.

import org.stellar.anchor.api.sep.sep45.ValidationRequest;
import org.stellar.anchor.api.sep.sep45.ValidationResponse;

public interface ISep45Service {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public interface ISep45Service {
public interface Sep45Service {

Unless ISep45Service is used somewhere else, we don't have to create the interface.

For ISep10Service, it was probably created for some legacy reasons that I can't remember. May be we may remove it as well.

import org.stellar.sdk.responses.sorobanrpc.SimulateTransactionResponse;

@Getter
public class Rpc {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public class Rpc {
public class StellarRpc {

Rpc is too generic.

@@ -40,6 +40,9 @@ stellar_network:
# The horizon server endpoint.
horizon_url: https://horizon-testnet.stellar.org

# The rpc server endpoint.
rpc_url: https://soroban-testnet.stellar.org
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
rpc_url: https://soroban-testnet.stellar.org
stellar_rpc_url: https://soroban-testnet.stellar.org

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These configurations are already nested under stellar_network so I think keeping it as rpc_url is fine.

@philipliu philipliu merged commit f21f62f into stellar:feature/c-accounts Feb 6, 2025
10 checks passed
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