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

Fork RocksDB into cosmos organization and use it in store/v2 #10263

Closed
5 of 6 tasks
robert-zaremba opened this issue Sep 29, 2021 · 11 comments · Fixed by #10430
Closed
5 of 6 tasks

Fork RocksDB into cosmos organization and use it in store/v2 #10263

robert-zaremba opened this issue Sep 29, 2021 · 11 comments · Fixed by #10430
Assignees
Labels

Comments

@robert-zaremba
Copy link
Collaborator

robert-zaremba commented Sep 29, 2021

Summary

We need a patch in RocksDB created by @roysc . Since Cosmos ecosystem is very active, we want to have a direct access to all dependencies we modify.

Ref: #9851 (comment)

  • Fork Go RocksDB into cosmos/rocksdb
  • update db/go.mod to use cosmos fork.

CC: @i-norden , @roysc


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

Do you need me to fork the repo under the cosmos org @robert-zaremba ?

@robert-zaremba
Copy link
Collaborator Author

Do you need me to fork the repo under the cosmos org @robert-zaremba ?

It would be preferred, however if you think it's not needed then we can close this task as noop.

@tac0turtle
Copy link
Member

Ill try to fork into cosmos, I think it makes the most sense

@tac0turtle
Copy link
Member

it has been forked. @roysc want to move over your changes?

@robert-zaremba
Copy link
Collaborator Author

This task was supposed to port roysc changes here. So the easy way was to ask @roysc simply push his version here.

Reopening until this is done, and subtasks resolved.

@robert-zaremba
Copy link
Collaborator Author

Also, we need to set the branch protection : merges in master should only go through PR. @okwme can you do that please?

@roysc
Copy link
Contributor

roysc commented Dec 6, 2021

@robert-zaremba can you clarify? what needs to be done besides updating https://github.com/cosmos/gorocksdb and the go.mod ref?

@tac0turtle
Copy link
Member

Also, we need to set the branch protection : merges in master should only go through PR. @okwme can you do that please?

added it, now one approval before merging

@tac0turtle
Copy link
Member

@robert-zaremba can you clarify? what needs to be done besides updating https://github.com/cosmos/gorocksdb and the go.mod ref?

I believe its to make the go.mod change only.

@robert-zaremba
Copy link
Collaborator Author

yes, go.mod

@roysc
Copy link
Contributor

roysc commented Dec 7, 2021

ok (that will be merged with #10430 - ad227e0)

@mergify mergify bot closed this as completed in #10430 Dec 16, 2021
mergify bot pushed a commit that referenced this issue Dec 16, 2021
## Description

Part of: #10192

Introduces a new `RootStore` type in the `store/v2` package and an implementation, without yet replacing the `MultiStore` or refactoring its use within the SDK (which will happen in the follow up: #10174).
Specified by [ADR-040](https://github.com/cosmos/cosmos-sdk/blob/1326fa2a7dfc3d83cf23dc1c1f33ff131152ad60/docs/architecture/adr-040-storage-and-smt-state-commitments.md).

Fixes #10651
Fixes #10263

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [x] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
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.

5 participants