-
Notifications
You must be signed in to change notification settings - Fork 558
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
fix(pkg/cosmosclient): ensure account has funds after faucet transfer #1846
Conversation
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
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.
👌🏽
only a question and a suggestion
@@ -384,7 +350,33 @@ func (c *Client) makeSureAccountHasTokens(ctx context.Context, address string) e | |||
} | |||
} | |||
|
|||
return nil | |||
// make sure funds are retrieved. | |||
ctx, cancel := context.WithTimeout(ctx, time.Minute) |
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.
Maybe 1 minute is it not too long to 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.
Is 5mins good? Tho, usually blocks are generated under 30secs, if your node is not behind of the other nodes in the network.
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'll update this to 2 mins and make in configurable.
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.
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 was trying to say 1 min is too much time. If we can reduce it to ~40 seconds can be better for the user experience.
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 see now, definitely.
this is useful if tx for faucet transfer not sent in the `sync` mode. * pkg/cosmosclient: improve faucet defaults. * pkg/cosmosfaucet: ensure that transfer tx uses `syncA mode.`
fce22f8
to
412c8bd
Compare
this is useful if tx for faucet transfer not sent in the
sync
mode.syncA mode.