Skip to content
This repository has been archived by the owner on Oct 27, 2020. It is now read-only.

Play with the verifier #37

Merged
merged 6 commits into from
Mar 7, 2020
Merged

Conversation

cwgoes
Copy link
Contributor

@cwgoes cwgoes commented Feb 24, 2020

Testing in progress.

fmt.Println("height:", height)
vs, err := store.ValidatorSet(height + 1)
vs, err := client.TrustedValidatorSet(height, time.Now())
Copy link
Member

Choose a reason for hiding this comment

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

NOTE: TrustedValSet takes the height and subtracts 1 so this should return the valset from height - 1

With the code the way it is now, we get the following error when updating clients:

go run main.go --home $RLY tx update-client ibc0 ibc1 ibconeclient
{"height":"0","txhash":"FBE550310E4DCEF7E2CD040D975E71DA41E9E13A88F55C05CD3AA9B93988184C","codespace":"client","code":9,"raw_log":"invalid block header: header blocktime can't be in the future: cannot update client with ID ibconeclient","gas_wanted":"200000","gas_used":"50242"}

If we change this to height-1 and the below call to height then we get the following error:

go run main.go --home $RLY tx update-client ibc0 ibc1 ibconeclient
{"height":"0","txhash":"BE0E95CBAF0AC40E3D0005DB41EE47D7BFFB26EF6BBEDCEE7414900B7A4B9B4A","codespace":"undefined","code":1,"raw_log":"internal error","gas_wanted":"200000","gas_used":"50262"}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just can't replicate this locally, at least not at the moment.

@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 26, 2020

So, at the moment, I can do:

./two-chains.sh

(set environment variables accordingly)

Setting clients works fine:

relayer --home $RLY tx clients ibc0 ibc1 ibconeclient ibczeroclient

The first connection-step works fine:

relayer --home $RLY tx connection-step ibc0 ibc1 ibconeclient ibczeroclient ibconeconn ibczeroconn

The second fails:

relayer --home $RLY tx connection-step ibc0 ibc1 ibconeclient ibczeroclient ibconeconn ibczeroconn
{"height":"6","txhash":"6B954F53057A24C2DB194BE30FAA7834AC88406E7476AA8E64BD96970E375253","codespace":"client","code":14,"raw_log":"connection state verification failed: key mismatch on operation #0: expected ibconeconn but got connection/ibconeconn: failed to execute message; message index: 1","gas_wanted":"200000","gas_used":"116000"}

Also, to even get to this error, I have to alter the error logic again - cosmos/cosmos-sdk@7465f9b.

@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 26, 2020

I'm not totally sure that my changes to the proof paths are correct; when I revert them I get:

{"height":"16","txhash":"E1D9E1CF1941043D0E5EC3BA49CF4F4A26AC460E6DE0CC20881BF85FB6067106","codespace":"client","code":14,"raw_log":"connection state verification failed: calculated root hash is invalid: expected [94 197 194 250 51 5 186 54 114 107 48 85 54 104 156 115 208 216 165 135 217 245 125 154 241 184 121 109 49 237 138 146] but got [28 0 255 77 7 105 199 212 24 235 180 224 30 86 54 56 202 188 117 51 97 98 167 201 44 223 63 151 73 178 147 38]: failed to execute message; message index: 1","gas_wanted":"200000","gas_used":"116003"}

(on the second connection step)

This error is after the key mismatch one though, per https://github.com/tendermint/tendermint/blob/master/crypto/merkle/proof.go#L60, so my earlier attempted fixes may not have fixed the problem anyways.

@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 26, 2020

Switching https://github.com/cosmos/cosmos-sdk/blob/ibc-alpha/x/ibc/23-commitment/merkle.go#L91 to KeyEncodingHex does not help, same mis-match.

@cwgoes
Copy link
Contributor Author

cwgoes commented Feb 26, 2020

I'm pretty lost here, do you have any ideas @AdityaSripal / does this match your debugging?

jackzampolin and others added 3 commits February 26, 2020 13:09
* refactor UpdateLiteDBsToLatestHeaders

* refactor GetLatestHeaders

* use channel

* close channel via defer

* update verifier to the latest lite version

ValidateBasic was removed because lite does this.

Co-authored-by: Jack Zampolin <[email protected]>
@jackzampolin jackzampolin merged commit 8a87afb into jack/debuggin Mar 7, 2020
@jackzampolin jackzampolin deleted the cwgoes/play-with-verifier branch March 7, 2020 17:59
jackzampolin added a commit that referenced this pull request Mar 10, 2020
* Update to latest ibc-alpha

* Fix build

* Validate header locally

* Fetch next validator set

* Fix minor typo

* Query latest height from client state

* Debuggin 🐛 🔍 👢

* Update to latest SDK

* Merge PR #37: Play with the verifier

* Use light client API directly

* Remove all direct store access

* Add some comments and fix two-chainz

* Update to latest SDK

* Merge PR #38: verifier: update to the latest Tendermint light client

* refactor UpdateLiteDBsToLatestHeaders

* refactor GetLatestHeaders

* use channel

* close channel via defer

* update verifier to the latest lite version

ValidateBasic was removed because lite does this.

Co-authored-by: Jack Zampolin <[email protected]>

Co-authored-by: Jack Zampolin <[email protected]>
Co-authored-by: Anton Kaliaev <[email protected]>

* Update sdk version

* Update twochains to run create and update client

* Add ability to print transaction before sending

* Call ValidateBasic() on header

* Merge PR #44: Fix hanging errors

* Back to current versions of the SDK and TM, working clients, WIP connections

* New error

* Update two chains to take us to error

* Results from debugging with Chris and Aditya

* Working connection-step and expanded ci-chains test

* --pruning=nothing on ci-chains.sh

* Update upstream version

* Merge PR #45: Working Connection handshake

Co-authored-by: Christopher Goes <[email protected]>
Co-authored-by: Anton Kaliaev <[email protected]>
jackzampolin pushed a commit that referenced this pull request Mar 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants