-
Notifications
You must be signed in to change notification settings - Fork 38
[ANCHOR-916] Put Horizon behind interface #1624
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
package org.stellar.anchor.ledger; | ||
|
||
import static org.stellar.anchor.api.asset.AssetInfo.NATIVE_ASSET_CODE; | ||
|
||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import lombok.Getter; | ||
import org.stellar.anchor.config.AppConfig; | ||
import org.stellar.anchor.ledger.LedgerTransaction.LedgerTransactionResponse; | ||
import org.stellar.anchor.util.AssetHelper; | ||
import org.stellar.sdk.*; | ||
import org.stellar.sdk.exception.NetworkException; | ||
import org.stellar.sdk.requests.PaymentsRequestBuilder; | ||
import org.stellar.sdk.responses.AccountResponse; | ||
import org.stellar.sdk.responses.TransactionResponse; | ||
import org.stellar.sdk.responses.operations.OperationResponse; | ||
import org.stellar.sdk.xdr.AssetType; | ||
|
||
/** The horizon-server. */ | ||
public class Horizon implements LedgerApi { | ||
|
||
@Getter private final String horizonUrl; | ||
@Getter private final String stellarNetworkPassphrase; | ||
private final Server horizonServer; | ||
|
||
public Horizon(AppConfig appConfig) { | ||
this.horizonUrl = appConfig.getHorizonUrl(); | ||
this.stellarNetworkPassphrase = appConfig.getStellarNetworkPassphrase(); | ||
this.horizonServer = new Server(appConfig.getHorizonUrl()); | ||
} | ||
|
||
public Server getServer() { | ||
return this.horizonServer; | ||
} | ||
|
||
public boolean hasTrustline(String account, String asset) throws NetworkException { | ||
String assetCode = AssetHelper.getAssetCode(asset); | ||
if (NATIVE_ASSET_CODE.equals(assetCode)) { | ||
return true; | ||
} | ||
String assetIssuer = AssetHelper.getAssetIssuer(asset); | ||
|
||
AccountResponse accountResponse = getServer().accounts().account(account); | ||
return accountResponse.getBalances().stream() | ||
.anyMatch( | ||
balance -> { | ||
TrustLineAsset trustLineAsset = balance.getTrustLineAsset(); | ||
if (trustLineAsset.getAssetType() == AssetType.ASSET_TYPE_CREDIT_ALPHANUM4 | ||
|| trustLineAsset.getAssetType() == AssetType.ASSET_TYPE_CREDIT_ALPHANUM12) { | ||
AssetTypeCreditAlphaNum creditAsset = | ||
(AssetTypeCreditAlphaNum) trustLineAsset.getAsset(); | ||
assert creditAsset != null; | ||
return creditAsset.getCode().equals(assetCode) | ||
&& creditAsset.getIssuer().equals(assetIssuer); | ||
} | ||
return false; | ||
}); | ||
} | ||
|
||
@Override | ||
public Account getAccount(String account) throws NetworkException { | ||
AccountResponse response = getServer().accounts().account(account); | ||
AccountResponse.Thresholds thresholds = response.getThresholds(); | ||
|
||
return Account.builder() | ||
.accountId(response.getAccountId()) | ||
.sequenceNumber(response.getSequenceNumber()) | ||
.thresholds( | ||
LedgerApi.Thresholds.builder() | ||
.lowThreshold(thresholds.getLowThreshold()) | ||
.medThreshold(thresholds.getMedThreshold()) | ||
.highThreshold(thresholds.getHighThreshold()) | ||
.build()) | ||
.balances( | ||
response.getBalances().stream() | ||
.map( | ||
b -> | ||
Balance.builder() | ||
.assetType(b.getAssetType()) | ||
.assetCode(b.getAssetCode()) | ||
.assetIssuer(b.getAssetIssuer()) | ||
.liquidityPoolId(b.getLiquidityPoolId()) | ||
.limit(b.getLimit()) | ||
.build()) | ||
.collect(Collectors.toList())) | ||
.signers( | ||
response.getSigners().stream() | ||
.map( | ||
s -> | ||
Signer.builder() | ||
.key(s.getKey()) | ||
.type(s.getType()) | ||
.weight(s.getWeight()) | ||
.sponsor(s.getSponsor()) | ||
.build()) | ||
.collect(Collectors.toList())) | ||
.build(); | ||
} | ||
|
||
@Override | ||
public LedgerTransaction getTransaction(String transactionId) throws NetworkException { | ||
TransactionResponse response = getServer().transactions().transaction(transactionId); | ||
return LedgerTransaction.builder() | ||
.hash(response.getHash()) | ||
.sourceAccount(response.getSourceAccount()) | ||
.envelopeXdr(response.getEnvelopeXdr()) | ||
.metaXdr(response.getResultMetaXdr()) | ||
Comment on lines
+106
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need these fields? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, we do. ObservedPayment.java There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we currently don't. Considering reducing the maintainance load, I will remove |
||
.sourceAccount(response.getSourceAccount()) | ||
.memo(response.getMemo()) | ||
.sequenceNumber(response.getSourceAccountSequence()) | ||
.createdAt(response.getCreatedAt()) | ||
.build(); | ||
} | ||
|
||
@Override | ||
public LedgerTransactionResponse submitTransaction(Transaction transaction) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to implement a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC, we discussed about having submission function that may be required if we are expanding the observer to txn submitter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we have this planned right now, so I think it makes sense to hold off on the implementation until then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The implementation was there and the tests needs it. We need submit for both RPC and horizon for testing. |
||
throws NetworkException { | ||
TransactionResponse txnR = getServer().submitTransaction(transaction, false); | ||
|
||
return LedgerTransactionResponse.builder() | ||
.hash(txnR.getHash()) | ||
.metaXdr(txnR.getEnvelopeXdr()) | ||
.envelopXdr(txnR.getEnvelopeXdr()) | ||
.sourceAccount(txnR.getSourceAccount()) | ||
.feeCharged(txnR.getFeeCharged().toString()) | ||
.createdAt(txnR.getCreatedAt()) | ||
.build(); | ||
} | ||
|
||
/** | ||
* Get payment operations for a transaction. | ||
* | ||
* @param stellarTxnId the transaction id | ||
* @return the operations | ||
* @throws NetworkException request failed, see {@link PaymentsRequestBuilder#execute()} | ||
*/ | ||
public List<OperationResponse> getStellarTxnOperations(String stellarTxnId) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, should this method be defined in the interface? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the plan to use the interface in the observer then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, if possible, I would use interface. If too different, then have to use classes for observers. |
||
return getServer() | ||
.payments() | ||
.includeTransactions(true) | ||
.forTransaction(stellarTxnId) | ||
.execute() | ||
.getRecords(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
package org.stellar.anchor.ledger; | ||
|
||
import java.io.IOException; | ||
import java.util.List; | ||
import lombok.Builder; | ||
import lombok.Getter; | ||
import lombok.Value; | ||
import org.stellar.sdk.KeyPair; | ||
import org.stellar.sdk.Transaction; | ||
import org.stellar.sdk.TransactionBuilderAccount; | ||
import org.stellar.sdk.exception.NetworkException; | ||
|
||
public interface LedgerApi { | ||
lijamie98 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Can we call this something else? Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have a better one?.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The functions are
I think How about |
||
/** | ||
* Check if the account has a trustline for the given asset. | ||
* | ||
* @param account The account to check. | ||
* @param asset The asset to check. | ||
* @return True if the account has a trustline for the asset. | ||
* @throws NetworkException If there was an error communicating with the network. | ||
*/ | ||
boolean hasTrustline(String account, String asset) throws NetworkException, IOException; | ||
|
||
/** | ||
* Get the account details for the given account. | ||
* | ||
* @param account The account to get. | ||
* @return The account details. | ||
* @throws NetworkException If there was an error communicating with the network. | ||
*/ | ||
Account getAccount(String account) throws NetworkException; | ||
|
||
/** | ||
* Get the operations for the given Stellar transaction. | ||
* | ||
* @param stellarTxnId The Stellar transaction ID. | ||
* @return The operations for the transaction. | ||
*/ | ||
LedgerTransaction getTransaction(String stellarTxnId); | ||
|
||
/** | ||
* Submit a transaction to the network. | ||
* | ||
* @param transaction The transaction to submit. | ||
* @return The transaction response. | ||
* @throws NetworkException If there was an error communicating with the network. | ||
*/ | ||
LedgerTransaction.LedgerTransactionResponse submitTransaction(Transaction transaction) | ||
throws NetworkException; | ||
|
||
@Builder | ||
@Getter | ||
class Account implements TransactionBuilderAccount { | ||
private String accountId; | ||
private Long sequenceNumber; | ||
|
||
private Thresholds thresholds; | ||
private List<Balance> balances; | ||
private List<Signer> signers; | ||
|
||
@Override | ||
public KeyPair getKeyPair() { | ||
return KeyPair.fromAccountId(accountId); | ||
} | ||
|
||
@Override | ||
public void setSequenceNumber(long seqNum) { | ||
sequenceNumber = seqNum; | ||
} | ||
|
||
@Override | ||
public Long getIncrementedSequenceNumber() { | ||
return sequenceNumber + 1; | ||
} | ||
|
||
/** Increments sequence number in this object by one. */ | ||
public void incrementSequenceNumber() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this used? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is required by the |
||
sequenceNumber++; | ||
} | ||
} | ||
|
||
@Builder | ||
@Getter | ||
class Thresholds { | ||
Integer lowThreshold; | ||
Integer medThreshold; | ||
Integer highThreshold; | ||
} | ||
|
||
@Builder | ||
@Getter | ||
class Balance { | ||
String assetType; | ||
String assetCode; | ||
String assetIssuer; | ||
String liquidityPoolId; | ||
String limit; | ||
String balance; | ||
} | ||
|
||
@Value | ||
@Builder | ||
@Getter | ||
class Signer { | ||
String key; | ||
String type; | ||
Integer weight; | ||
String sponsor; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we only return the account signers in this method? The
balances
field is already covered by thehasTrustlines
method. Implementing this RPC would mean making twoGetLedgerEntries
calls when most of the time, we would only care about signers or trustlines/balances but not both.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is based on RPC ledger endpoint. It returns both
signers
andtrustlines
.The
GetLedgerEntries
takes aList<LedgerKey>
. This is only one call though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the
hasTrustline
should be a helper function instead of a interface.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that's right. Do we need balance, though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I will remove.