-
Notifications
You must be signed in to change notification settings - Fork 6
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
Execute payout transactions #127
Conversation
@mpetrun5 @mpetrunic I made PR on |
Can we have same changes in our fork so that we dont depend on upstream? |
@MakMuftic Is this still draft? |
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.
We need better docs, add following to readme:
- where to obtain test dots, to what address they need to be sent
- how to trigger payment early payment
- how to set periodic payments
What happens if node sent invalid address? Is fail tx reported with address, node and amount?
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.
Probably will be easier to test some stuff when refactored
…ecute-transactions
ready for review? @MakMuftic |
I think it is, additional testing is definitely going to be needed but I think we can merge this and resolve any additional problems in separate PRs. |
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.
When starting add log with some human readable date when next payout will be. If you have some regular checks whether payout should be maybe, you could add that log there as well. Critical log if for some reason payout wasn't made
Co-authored-by: Matija Petrunić <[email protected]>
…Io/vedran into mmuftic/execute-transactions
I extended payout logs a bit. I like this idea of a regular log for when will the next payout be and I will add it when resolving #134 as this is tightly connected (I added a reminder for this in issue) If the payout fails here is a snippet of what is being logged transactionDetails, err := script.ExecutePayout(secret, reward, loadBalancerUrl)
if transactionDetails != nil {
// display even if only part of transactions executed
ui.DisplayTransactionsStatus(transactionDetails)
}
if err != nil {
log.Errorf("Unable to execute payout, because of: %v", err)
return
} else {
log.Info("Payout execution finished")
} I think this is enough information @mpetrunic ?, but what is a problem (edge case) that we are still not addressing is this one: What if something fails while the payout is being executed, then we could have a scenario where only a subset of payout transaction was executed. Just something to think about, we will not handle this in this PR. |
Expand payout command so it executes all payout transactions
PR Checklist
Changes
github.com/centrifuge/go-substrate-rpc-client
Issues
Closes #107
Closes #121
TODO
I am working on our fork ofgithub.com/centrifuge/go-substrate-rpc-client
as I realized it is still less work to refactor this to use go port of subkey than to write everything from scratch.