search

cosmos / iavl

Merkleized IAVL+ Tree implementation in Go
Apache License 2.0
422 stars 263 forks source link

feat: Change the v0.20.x line to use cosmos DB #768

Closed ValarDragon closed 1 year ago

ValarDragon commented 1 year ago

This is done so we can merge #765

ValarDragon commented 1 year ago

This CI lint seems wrong? I tried changing both of the nolint's in the codebase, but they do warn for a gocritic problem. (Also these are unchanged by this PR)

ValarDragon commented 1 year ago

cc @tac0turtle or anyone who worked on the initial cosmos DB move, any idea why benchmarks is getting build errors for rocksDB and how to fix it?

(Also is this something thats acceptable to merge into v0.20.x line?)

tac0turtle commented 1 year ago

I can look into this. We have different versions for things due to the comet migration so it's confusing.

julienrbrt commented 1 year ago

I can look into this. We have different versions for things due to the comet migration so it's confusing.

I was looking as well lol -> https://github.com/cosmos/iavl/pull/769 I think we cannot because this will break v0.47.x users.

ValarDragon commented 1 year ago

oh yikes thats super confusing. OK :/

Makes sense with the incompatibalities due to tendermint vs comet vs cosmos. Guess I have to revive the IAVL fork until Osmosis updates its SDK.

Or maybe I make a more questionable version of this batch size tracking for this release line.

tac0turtle commented 1 year ago

@ValarDragon which version of the sdk do you need this for? we can make it work for your version and use a wrapper if neede

ValarDragon commented 1 year ago

Its for SDK v0.45! But I think it shouldn't be a big deal to make a more complicated IAVL change here to track the batch size in IAVL instead of the DB layer for this release line.

tac0turtle commented 1 year ago

lets backport to 0.21 and the app can have a wrapper around the db for use with 0.45