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

feat: ListWalletTransactions rpc #286

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jackstar12
Copy link
Member

No description provided.

@jackstar12 jackstar12 force-pushed the list-wallet-transactions branch 2 times, most recently from 768ece6 to 357d4f2 Compare September 11, 2024 19:51
boltzrpc/boltzrpc.proto Outdated Show resolved Hide resolved
boltzrpc/boltzrpc.proto Outdated Show resolved Hide resolved
boltzrpc/boltzrpc.proto Outdated Show resolved Hide resolved
boltzrpc/boltzrpc.proto Outdated Show resolved Hide resolved
database/any.go Outdated Show resolved Hide resolved
Copy link
Member

@michael1011 michael1011 left a comment

Choose a reason for hiding this comment

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

Misclicked the approve

@jackstar12 jackstar12 force-pushed the list-wallet-transactions branch 2 times, most recently from 534d3b8 to f72a88c Compare September 15, 2024 12:56
@jackstar12 jackstar12 marked this pull request as ready for review September 16, 2024 10:45
Copy link
Member

@michael1011 michael1011 left a comment

Choose a reason for hiding this comment

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

utACK

@@ -733,6 +738,49 @@ message GetWalletRequest {
optional uint64 id = 2;
}

message ListWalletTransactionsRequest {
uint64 id = 1;
Copy link
Member

Choose a reason for hiding this comment

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

What does id mean in this context here? It's not clear from just the proto

string address = 1;
uint64 amount = 2;
// wether the address is controlled by the wallet
bool is_our_address = 3;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bool is_our_address = 3;
bool is_ours = 3;

// its the sum of all output values minus the sum of all input values which are controlled by the wallet.
// positive values indicate incoming transactions, negative values outgoing transactions
int64 balance_change = 2;
int64 timestamp = 3;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int64 timestamp = 3;
uint64 timestamp = 3;

return nil, err
}
for _, tx := range outputs.Transactions {
if tx.TxId == txId || txId == "" {
for _, tx := range transactoins {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for _, tx := range transactoins {
for _, tx := range transactions {

limit = 30
}
if limit > 30 {
return nil, errors.New("limit cant be larger than 30")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return nil, errors.New("limit cant be larger than 30")
return nil, errors.New("limit cannot be larger than 30")

message WalletTransaction {
string id = 1;
// balance change of the wallet in satoshis.
// its the sum of all output values minus the sum of all input values which are controlled by the wallet.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// its the sum of all output values minus the sum of all input values which are controlled by the wallet.
// It is the sum of all output values minus the sum of all input values which are controlled by the wallet.


message WalletTransaction {
string id = 1;
// balance change of the wallet in satoshis.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// balance change of the wallet in satoshis.
// Balance change of the wallet in satoshis.

string id = 1;
// balance change of the wallet in satoshis.
// its the sum of all output values minus the sum of all input values which are controlled by the wallet.
// positive values indicate incoming transactions, negative values outgoing transactions
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// positive values indicate incoming transactions, negative values outgoing transactions
// Positive values indicate incoming transactions, negative values outgoing transactions.

int64 timestamp = 3;
repeated TransactionOutput outputs = 4;
uint32 block_height = 6;
// swaps which are related to this transaction
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// swaps which are related to this transaction
// Swaps which are related to this transaction.

message TransactionOutput {
string address = 1;
uint64 amount = 2;
// wether the address is controlled by the wallet
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// wether the address is controlled by the wallet
// Whether the address is controlled by the wallet.

a bit flaky with gdk notifications test helper - nothing to worry about in prod
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