-
Notifications
You must be signed in to change notification settings - Fork 103
accounts: add functionality to update balance by given amount #962
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
32eb4c5
21c0677
8c24c1a
755068b
3cd1569
078ca5f
a01233b
debfa05
54005b0
aed7ef3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -132,6 +132,53 @@ func (s *RPCServer) UpdateAccount(ctx context.Context, | |
return marshalAccount(account), nil | ||
} | ||
|
||
// UpdateBalance adds or deducts an amount from an existing account in the | ||
// account database. | ||
func (s *RPCServer) UpdateBalance(ctx context.Context, | ||
req *litrpc.UpdateAccountBalanceRequest) (*litrpc.Account, error) { | ||
|
||
var ( | ||
isAddition bool | ||
amount lnwire.MilliSatoshi | ||
) | ||
|
||
switch reqType := req.GetUpdate().(type) { | ||
case *litrpc.UpdateAccountBalanceRequest_Add: | ||
log.Infof("[addbalance] id=%s, label=%v, amount=%d", | ||
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 do you think of using "credit/debit" instead of "add/deduct"? 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. Ah yeah definitely! I was struggling with naming, and agree that credit & debit is better. |
||
req.Id, req.Label, reqType.Add) | ||
|
||
isAddition = true | ||
amount = lnwire.MilliSatoshi(reqType.Add * 1000) | ||
|
||
case *litrpc.UpdateAccountBalanceRequest_Deduct: | ||
log.Infof("[deductbalance] id=%s, label=%v, amount=%d", | ||
req.Id, req.Label, reqType.Deduct) | ||
|
||
isAddition = false | ||
amount = lnwire.MilliSatoshi(reqType.Deduct * 1000) | ||
} | ||
|
||
if amount <= 0 { | ||
return nil, fmt.Errorf("amount %v must be greater than 0", | ||
int64(amount/1000)) | ||
} | ||
Comment on lines
+161
to
+164
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. why not use uints in the proto definitions 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. Good idea! Addressed in the new PR. |
||
|
||
accountID, err := s.findAccount(ctx, req.Id, req.Label) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Ask the service to update the account. | ||
account, err := s.service.UpdateBalance( | ||
ctx, accountID, amount, isAddition, | ||
) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return marshalAccount(account), nil | ||
} | ||
|
||
// ListAccounts returns all accounts that are currently stored in the account | ||
// database. | ||
func (s *RPCServer) ListAccounts(ctx context.Context, | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
just an idea for standard PR flow: we always try to (as much as possible at least) to keep commits ordered such that things are working commit by commit. For 2 reasons: 1) reverting back to any single commit isnt a problem and 2) review story telling.
so the ideal flow for RPC additions is:
the high level pattern im describing here is:
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.
Thanks a lot for the suggestion! I've tried to follow this with the introduction of the new #973 & #974 PRs.