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

[Feature]: Bring back block inclusion flag for txs #18927

Closed
faboweb opened this issue Jan 2, 2024 · 10 comments
Closed

[Feature]: Bring back block inclusion flag for txs #18927

faboweb opened this issue Jan 2, 2024 · 10 comments
Assignees

Comments

@faboweb
Copy link
Contributor

faboweb commented Jan 2, 2024

Summary

Before you could -b block so the tx send via the CLI would wait for the tx to be included and then return the result. This got removed.

Problem Definition

Having shell scripts around the CLI got a lot more complex because of this removal.

Proposed Feature

Bring back the same feature as there was before.

@github-project-automation github-project-automation bot moved this to 👀 To Do in Cosmos-SDK Jan 2, 2024
@tac0turtle tac0turtle self-assigned this Jan 2, 2024
@tac0turtle tac0turtle moved this from 👀 To Do to ✍ In Progress in Cosmos-SDK Jan 2, 2024
@tac0turtle
Copy link
Member

Hey, this was removed due to block being made for testing and having bled into production. Unfortunately, we wont be adding it back. We recommend using a js library like cosmjs to make your life easier or using a wallet

@github-project-automation github-project-automation bot moved this from ✍ In Progress to 🥳 Done in Cosmos-SDK Jan 2, 2024
@faboweb
Copy link
Contributor Author

faboweb commented Jan 2, 2024

Can you elaborate why it is an issue having this feature? Other teams (Osmosis) also complained.
It feels like a very simple feature to have with high impact.

@ValarDragon
Copy link
Contributor

Would really appreciate this being added back as well. This automates the most common setup flow, and works great under the happy path

@julienrbrt
Copy link
Member

I believe Crypto.com added a command that would do pretty much the same thing.

@alexanderbez
Copy link
Contributor

The command/usage is faulty because often times you'd get an RPC error since block inclusion can take much longer than the RPC timeouts. In testing environments, this can be easily controlled to an extent, but in practical environments, it's not really reliable.

@julienrbrt
Copy link
Member

Found the command alternative from cdc: #17274

@faboweb
Copy link
Contributor Author

faboweb commented Jan 3, 2024

The command/usage is faulty because often times you'd get an RPC error since block inclusion can take much longer than the RPC timeouts. In testing environments, this can be easily controlled to an extent, but in practical environments, it's not really reliable.

Can't you handle RPC timeout and block inclusion separately? So instead of waiting, keep polling?

@faboweb
Copy link
Contributor Author

faboweb commented Jan 3, 2024

Found the command alternative from cdc: #17274

This looks cool. This only got added recently? Can't find the command in latest Osmosis.

@alexanderbez
Copy link
Contributor

alexanderbez commented Jan 3, 2024

The command/usage is faulty because often times you'd get an RPC error since block inclusion can take much longer than the RPC timeouts. In testing environments, this can be easily controlled to an extent, but in practical environments, it's not really reliable.

Can't you handle RPC timeout and block inclusion separately? So instead of waiting, keep polling?

No because broadcasting txs happens via CometBFT's BroadcastTx RPC method -- this will inevitably timeout. The only thing you can do is Broadcast (sync or async) + Poll/Wait, which you can write a tool for. There's also a command @julienrbrt linked.

@julienrbrt
Copy link
Member

julienrbrt commented Jan 3, 2024

Found the command alternative from cdc: #17274

This looks cool. This only got added recently? Can't find the command in latest Osmosis.

Should be available from v0.47.5. Maybe osmosis didn't add it to their app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants