-
Notifications
You must be signed in to change notification settings - Fork 28
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
pool: refactor payDividends. #250
Conversation
ba62f16
to
e682103
Compare
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.
I have reviewed the changes in paymentmgr.go
, but I have not yet looked at the tests - I will review those after these comments are addressed.
e682103
to
93010ea
Compare
This updates payment manager related errors to leverage the updated error types.
This refactors payDividends into smaller scoped helper functions that are easier to test.
This adds tests for payDividends and its helper functions.
This adds the --conftimeout config option to allow for custom tx confirmation timeouts.
This upates the tx confirmation streaming process to be non-blocking in order to terminate cleanly on context cancellation.
93010ea
to
9828e99
Compare
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 a few minor things in the test file and then this looks good to merge.
initialAccountPayments := xValue + yValue | ||
updatedAccountPaymentsPlusTxFee := out[xAddr] + out[yAddr] + txFee | ||
if initialAccountPayments-updatedAccountPaymentsPlusTxFee <= maxRoundingDiff { | ||
t.Fatalf("initial account payment total %v to be equal to updated "+ |
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.
cancel()
?
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, applyTxFees
is not using the context, therefore does not need the context cancelled for a clean shutdown.
This refactors the
payDividends
function into more targeted helper functions that are easier to test.