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-916] Put Horizon behind interface #1624

Open
wants to merge 1 commit into
base: epic/anchor-937-stellar-rpc-support
Choose a base branch
from

Conversation

lijamie98
Copy link
Collaborator

Description

  • Put Horizon class behind the LedgerApi interface

Context

  • Stellar RPC integration

Testing

  • ./gradlew test

Documentation

N/A

Known limitations

N/A

@lijamie98 lijamie98 force-pushed the feature/anchor-916-put-horizon-behind-interface branch from f5dca97 to 463ae5b Compare January 24, 2025 01:24
* @return The transaction response.
* @throws NetworkException
*/
TransactionResponse submitTransaction(Transaction transaction) throws NetworkException;
Copy link
Contributor

Choose a reason for hiding this comment

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

TransactionResponse is Horizon's submitTransaction response. RPC's sendTransaction returns a SendTransactionResponse which only contains the transaction hash and its status. We will need to call getTransaction to fetch the transaction afterward. The schemas are not the same. Should we create a new type to encapsulate the transaction submission responses?

import org.stellar.sdk.responses.TransactionResponse;
import org.stellar.sdk.responses.operations.OperationResponse;

public interface LedgerApi {
Copy link
Contributor

Choose a reason for hiding this comment

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

SEP-45 uses transaction simulation which is only available in RPC. How do we plan to expose RPC-specific methods in the interface?

* @param stellarTxnId The Stellar transaction ID.
* @return The operations for the transaction.
*/
List<OperationResponse> getStellarTxnOperations(String stellarTxnId);
Copy link
Contributor

Choose a reason for hiding this comment

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

OperationResponse is Horizon's getOperation response. If we use RPC, we must parse the operations from the transaction envelope. Should we create a new type to encapsulate the operation response?

private AccountEntry fetchAccountEntry(String account) throws IOException {
// TODO: Implement this method
return null;
// GetLedgerEntriesResponse response;
Copy link
Contributor

Choose a reason for hiding this comment

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

Test code?

@@ -71,4 +115,9 @@ public List<OperationResponse> getStellarTxnOperations(String stellarTxnId) {
.execute()
.getRecords();
}

@Override
public TransactionResponse submitTransaction(Transaction transaction) throws NetworkException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we only submit transactions in tests? It should not be part of the interface if that's the case.

}

/** Increments sequence number in this object by one. */
public void incrementSequenceNumber() {
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this used?

@philipliu
Copy link
Contributor

Also, for the branch name. I think we typically call long-lived feature branches feature/.... There is branch protection on any branch that starts with that prefix.

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