Skip to content
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

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

ilgooz
Copy link
Member

@ilgooz ilgooz commented Nov 29, 2021

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.

@ilgooz ilgooz linked an issue Nov 29, 2021 that may be closed by this pull request
@ilgooz ilgooz requested a review from noandrea November 29, 2021 13:10
Copy link
Contributor

@noandrea noandrea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@Pantani Pantani left a 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)
Copy link
Collaborator

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?

Copy link
Member Author

@ilgooz ilgooz Nov 30, 2021

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.

Copy link
Member Author

@ilgooz ilgooz Nov 30, 2021

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

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.

Copy link
Member Author

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.`
@ilgooz ilgooz merged commit 3bb9210 into master Nov 30, 2021
@ilgooz ilgooz deleted the fix/cosmosclient branch November 30, 2021 10:44
@lumtis lumtis mentioned this pull request Nov 30, 2021
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cosmosclient: investigate account not found error
4 participants