Skip to content

Create tendermint client#1255

Merged
antho1404 merged 3 commits intodevfrom
cosmos/tmclient
Aug 24, 2019
Merged

Create tendermint client#1255
antho1404 merged 3 commits intodevfrom
cosmos/tmclient

Conversation

@krhubert
Copy link
Copy Markdown
Contributor

@krhubert krhubert commented Aug 23, 2019

Create a simple helper that could be used as tendermint client

@krhubert krhubert added the enhancement New feature or request label Aug 23, 2019
@krhubert krhubert added this to the next milestone Aug 23, 2019
@krhubert krhubert self-assigned this Aug 23, 2019
Comment thread cosmos/client/client.go
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()

out, err := c.Subscribe(ctx, "", "tx.hash='"+txres.Hash.String()+"'")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just to make sure, this subscribe is only to get the result of the txhash but we already have the txres so why do we need to subscribe?
If this subscribe is to wait for the end of the transaction, a timeout of 20 seconds might be too short.
If this subscribe is for some reason get the transaction isn't it a problem that we subscribe after broadcasting? what happens if it's too fast and we didn't subscribe yet? especially that there is a BroadcastTxSync

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the first draft of this system. We need to use it to see the best way to implement this ;)

@mesg-bot
Copy link
Copy Markdown
Member

This pull request has been mentioned on MESG Community. There might be relevant details there:

https://forum.mesg.com/t/network-implementation-with-cosmossdk/304/15

@antho1404 antho1404 merged commit ad56036 into dev Aug 24, 2019
@antho1404 antho1404 deleted the cosmos/tmclient branch August 24, 2019 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants