-
Notifications
You must be signed in to change notification settings - Fork 122
staticaddr: minor fixes #956
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
Conversation
5b287c8 to
9f7726a
Compare
staticaddr/deposit/manager.go
Outdated
|
|
||
| numRecoveredDeposits++ | ||
| } | ||
| wg.Wait() |
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 don't think this commit makes sense. If we recover into the action waitforexpirysweptaction this recover will block until the tx is confirmed, making loop unusable for uncertain block times.
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.
A solution would be to make the action non-blocking (e.g. by moving receiving from the notifiers into a goroutine and sending an event to the fsm)
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.
Thank you for the catch @sputn1ck. I removed this commit.
9f7726a to
22266d6
Compare
starius
left a comment
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.
LGTM!
Not sure if the second commit is needed.
|
|
||
| m.activeLoopIns[swapHash] = fsm | ||
| }() | ||
| }(fsm, loopIn.SwapHash) |
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 needed? fsm and swapHash are a local variable and they can't change outside of the goroutine.
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.
loopIn is a iteration variable which might change before the goroutine launches, so this is needed.
22266d6 to
80808b6
Compare
80808b6 to
d526192
Compare
depends on #954, ignore first commit