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

Can we skip polling when using broadcastTx ? #1316

Closed
williamchong opened this issue Oct 31, 2022 · 7 comments
Closed

Can we skip polling when using broadcastTx ? #1316

williamchong opened this issue Oct 31, 2022 · 7 comments

Comments

@williamchong
Copy link

Currently broadcastTx always polls tx for block inclusion. While timeoutMs and pollIntervalMs is configurable, setting them to 0 would make the function instantly throws a timeout error as a side effect.
Allowing polling to be skipped in broadcastTx() without throwing an error would allow batch sending of many transactions to be easier.

(I understand many messages can be included in one transaction, however in some cases we would want to send batch transactions with different memos)

@webmaster128
Copy link
Member

Tracking the block inclusion is an essential feature of broadcastTx. If you don't need or want that feature, it buils down to just this line:

const broadcasted = await this.forceGetTmClient().broadcastTxSync({ tx });

which you can call directly if you have a Tendermint34Client at hand. You can do that manually.

An alternative approach to using Tendermint34Client is keeping the polling but not waiting for the promise resolutions. In https://github.com/noislabs/nois-bot/blob/74a5f9c8fc64c325f0d078420169a8514e3c3dbf/index.js#L205-L222 you find a multi-cast implementation where 3 broadcastTx to different RPCs are made in parallel. Once everything is sent, the code checks all the results.

@williamchong
Copy link
Author

it's actually hard to get Tendermint34Client from stargateSigningClient since it's a private getter
To make usage easier, do you think skipping polling if timeoutMs is set to special value 0 or -1 makes sense? If yes I can try to make a PR

@webmaster128
Copy link
Member

What if we add a broadcastTxSync next to broadcastTx which does not track the block inclusion? It would only contain those lines and then return after checkTx:

const broadcasted = await this.forceGetTmClient().broadcastTxSync({ tx });
if (broadcasted.code) {
return Promise.reject(
new BroadcastTxError(broadcasted.code, broadcasted.codespace ?? "", broadcasted.log),
);
}
const transactionId = toHex(broadcasted.hash).toUpperCase();

@webmaster128 webmaster128 added this to the 0.30.1 milestone Mar 7, 2023
@williamchong
Copy link
Author

Yes that would work well 👍

@webmaster128
Copy link
Member

@williamchong Can you have a look at #1396 to see if it solves your problem?

@williamchong
Copy link
Author

@williamchong Can you have a look at #1396 to see if it solves your problem?

Yes it would, thanks

@webmaster128
Copy link
Member

Done in #1396

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

2 participants