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

Client context uses singleton lite verifier #5116

Closed
mossid opened this issue Oct 1, 2019 · 3 comments · Fixed by #5136
Closed

Client context uses singleton lite verifier #5116

mossid opened this issue Oct 1, 2019 · 3 comments · Fixed by #5136
Assignees
Labels

Comments

@mossid
Copy link
Contributor

mossid commented Oct 1, 2019

context.CLIContext.verifier is defined as a reference to the global singleton lite verifier, assuming that the process will connect to only a single chain. In https://github.com/cosmos/cosmos-sdk/blob/master/client/context/context.go#L77

var (
	verifier     tmlite.Verifier
	verifierHome string
)


	// We need to use a single verifier for all contexts
	if verifier == nil || verifierHome != viper.GetString(flags.FlagHome) {
		verifier = createVerifier()
		verifierHome = viper.GetString(flags.FlagHome)
	}

This is not true on IBC, where the handshake/packet handling requires access on both chains. We can keep it as a global verifier but it has to be map[string]tmlite.Verifier where the key is verifierHome. This way we can let the multiple contexts share the same verifier if they are connecting to the same chain but can have different verifiers if not.

@alexanderbez alexanderbez added x/ibc and removed T:Bug labels Oct 1, 2019
@alexanderbez alexanderbez added this to the IBC Implementation & Integration milestone Oct 1, 2019
@alexanderbez alexanderbez self-assigned this Oct 1, 2019
@melekes
Copy link
Contributor

melekes commented Oct 1, 2019

It might be a good idea to try tendermint/tendermint#3989 where verify is a pure function as opposite to Verifier struct.

@alexanderbez
Copy link
Contributor

It seems that's a better approach. I think in order for the ibc testnet to run successfully we'll need something quick so we can quickly tackle this and then remove it once tendermint/tendermint#3989 is ready and in a release.

@alexanderbez
Copy link
Contributor

After discussing with @melekes I think these are somewhat orthogonal. I will continue on with the proposed solution for which we can use the IBC-Gaia integration testnet.

@fedekunze fedekunze modified the milestones: IBC Implementation & Integration, IBC Implementation Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants