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

Remove cosmos/iavl from lazyledger-core to stop dependency graph cycle #128

Closed
evan-forbes opened this issue Jan 9, 2021 · 3 comments · Fixed by #129
Closed

Remove cosmos/iavl from lazyledger-core to stop dependency graph cycle #128

evan-forbes opened this issue Jan 9, 2021 · 3 comments · Fixed by #129
Assignees
Labels
T:Bug Type: Bug (confirmed) T:dependencies Type: Pull requests that update a dependency file

Comments

@evan-forbes
Copy link
Member

evan-forbes commented Jan 9, 2021

Currently, we're trying to replace tendermint for lazyledger-core in the lazyerleger-app #4 and in the lazyledger fork of the cosmos-sdk #2. We run into the same error if we attempt to change the import by using either the go modules' replace directive, or by using a script to manually replace every import from "github.com/tendermint/tendermint" to "github.com/lazyledger/lazyledger-core".
error:

go: github.com/cosmos/cosmos-sdk/baseapp imports
        github.com/tendermint/tendermint/abci/types imports
        github.com/lazyledger/lazyledger-core/crypto/ed25519: github.com/lazyledger/[email protected]: parsing go.mod:
        module declares its path as: github.com/tendermint/tendermint
                but was required as: github.com/lazyledger/lazyledger-core

After digging through the dependency trees of the libraries, I found that lazyledger-core and the cosmos sdk are both importing different versions of the cosmos/iavl, and those two versions of the iavl each import their own version of tendermint. So, connecting the cosmos-sdk to lazyledger-core brings us to a special level of dependency hell, go.mod dependency hell, which stops go modules from reconciling the different module declariation of tendermint and its fork, lazyledger-core. better explained in this post.

Screenshot from 2021-01-08 19-25-40

I think we're going to have to simultaneously update our fork of the cosmos-sdk, lazyledger-core, and a new fork of cosmos/iavl to new non-existent versions in order to break the import cycle. as described here

@evan-forbes
Copy link
Member Author

evan-forbes commented Jan 9, 2021

I also found another import cycle that stops go modules from updating the modules of lazyledger-core with go get -u ./..., that's caused by the recent switch from github.com/liamsi/merkletree to github.com/lazyledger/merkletree. Luckily, the import cycle is simpler, and can be fixed by updating the nmt module imported in lazyledger-core using go get -u github.com/lazyledger/nmt.
Screenshot from 2021-01-08 18-43-29

@tac0turtle
Copy link
Contributor

Within lazyledger-core we can remove Iavl. It is used for testing light client queries but we will not be supporting these form of proofs (I believe). I can open a PR later today.

@evan-forbes evan-forbes changed the title Dependency graph is stopping the cosmos-sdk from being able to import lazyledger-core Remove cosmos/iavl from lazyledger-core to stop dependency graph cycle Jan 9, 2021
@liamsi
Copy link
Member

liamsi commented Jan 9, 2021

It makes sense to remove IAVL from ll-core. AFAR, it is only used in tests only.

Linking related issues & discussions here for context:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:Bug Type: Bug (confirmed) T:dependencies Type: Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants