-
Notifications
You must be signed in to change notification settings - Fork 1
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
services: rpc payment ingestor #47
Conversation
} | ||
|
||
cmd := &cobra.Command{ | ||
Use: "ingest", | ||
Short: "Run Ingestion service", | ||
PersistentPreRunE: func(cmd *cobra.Command, args []string) error { | ||
// SET UP WEBHOOK CHANNEL HERE |
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.
Is this a TODO
?
PersistentPostRun: func(_ *cobra.Command, _ []string) { | ||
}, |
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.
Why do we need this?
func RPCURLOption(configKey *string) *config.ConfigOption { | ||
return &config.ConfigOption{ | ||
Name: "rpc-url", | ||
Usage: "The URL of the RPC Server.", | ||
OptType: types.String, | ||
ConfigKey: configKey, | ||
FlagDefault: "localhost:8080", | ||
Required: true, | ||
} | ||
} |
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.
What do you think about validating if the value passed here is a URL using the CustomSetValue
?
if txMemo != nil { | ||
*txMemo = utils.SanitizeUTF8(*txMemo) | ||
} | ||
|
||
for idx, op := range tx.Envelope.Operations() { | ||
txEnvelopeXDR.SourceAccount() |
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.
Perhaps we can remove this line.
txns, err := ingestService.GetLedgerTransactions(1) | ||
assert.Equal(t, 1, len(txns)) | ||
assert.Equal(t, txns[0].Hash, "hash1") | ||
assert.Empty(t, err) |
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.
assert.Empty(t, err) | |
assert.NoError(t, err) |
@@ -52,8 +56,33 @@ func (r *rpcService) GetTransaction(transactionHash string) (entities.RPCGetTran | |||
return result, nil | |||
} | |||
|
|||
func (r *rpcService) GetTransactions(startLedger int64, startCursor string, limit int) (entities.RPCGetTransactionsResult, error) { | |||
if limit > PageLimit { | |||
return entities.RPCGetTransactionsResult{}, fmt.Errorf("limit cannot exceed") |
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.
Perhaps we can add a limit to the error to make it clearer.
) | ||
|
||
type RPCTXCode struct { | ||
TxResultCode xdr.TransactionResultCode | ||
OtherCodes OtherCodes | ||
} | ||
|
||
var FinalErrorCodes = []xdr.TransactionResultCode{ | ||
xdr.TransactionResultCodeTxSuccess, |
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.
Why is this value on FinalErrorCodes
slice?
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.
Perhaps we can rename it to FinalCodes
only or FinalStateCodes
.
account := op.SourceAccount | ||
if account != nil { | ||
return account.ToAccountId().Address() | ||
} | ||
|
||
return tx.Envelope.SourceAccount().ToAccountId().Address() | ||
txEnvelope.SourceAccount() |
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.
Perhaps we can remove this line.
What
This pr replaces the ingestion of payments with a getTransactions RPC call
Why
We are removing all dependency on core in the wallet bakend, replacing it with rpc instead
Known limitations
N/A
Issue that this PR addresses
Checklist
PR Structure
all
if the changes are broad or impact many packages.Thoroughness
Release