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

feat: merge the QGB v0.46.x branch to the default one #276

Merged
merged 12 commits into from
Sep 19, 2022

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented Sep 16, 2022

Merge the QGB feature branch to the default branch in preparation for the QGB app merge.

* adds orchestrator address to validator initialization in staking module

* fix msg edit  validator

* todo

* todo
…r struct (#133)

* first pass on tests fixes

* fixes the rest of unit tests

* remove unnecessary comments

* uses default eth address when starting sim network

* cosmetics

* comments failing test

* comments failing test

* uncomments test and fixes it from commit 434b308

* remove wrong testnet initialization
* first pass on tests fixes

* fixes the rest of unit tests

* remove unnecessary comments

* uses default eth address when starting sim network

* cosmetics

* comments failing test

* comments failing test

* adds orchestrator/ethereum address checks for validators when creating and editing

* uses correct error codes for eth/orch address errors

* uncomments test and fixes it from commit 434b308

* adds zero eth address check when creating/updating validator

* attempts to fix duplicate eth address in sim network

* revert squashed changes

* uses unwrapped context for msg_server orch/eth validation

* increase DefaultGenTxGas to accomodate new qgb validator changes

* scaffolds an ethereum address from orchestrator address in staking simulation to have a deterministic way of creating it

* fix eth address creation from orch address in operations

* increase DefaultGenTxGas

* increase DefaultGenTxGas

* simpler way of creating an eth address from orch address in operations
* first pass on tests fixes

* fixes the rest of unit tests

* remove unnecessary comments

* uses default eth address when starting sim network

* cosmetics

* comments failing test

* comments failing test

* adds orchestrator/ethereum address checks for validators when creating and editing

* uses correct error codes for eth/orch address errors

* uncomments test and fixes it from commit 434b308

* updates tests to be able to set orch/eth address when creating validators

* adds zero eth address check when creating/updating validator

* attempts to fix duplicate eth address in sim network

* adds validator check tests

* revert squashed changes

* uses unwrapped context for msg_server orch/eth validation

* increase DefaultGenTxGas to accomodate new qgb validator changes

* scaffolds an ethereum address from orchestrator address in staking simulation to have a deterministic way of creating it

* fix eth address creation from orch address in operations

* increase DefaultGenTxGas

* increase DefaultGenTxGas

* remove unnecessary eth address
* remove EthAddress and use geth address

* adding checks on ethereum address when creating it

* fix ethereum address init when creating validator

* fix edit validator
go mod tidy

go mod tidy

remove unnecessary change

remove unnecessary else branch

fix gentx test

update staking.pb.go

fix staking cli tests

fixing tests: first pass

proto-gen
…ial-probably

chore!: Upgrade QGB branch to v0.46.0
@rach-id rach-id added enhancement New feature or request C: app Changes related to the celestia-app branches labels Sep 16, 2022
@rach-id rach-id self-assigned this Sep 16, 2022
Copy link
Member Author

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

cosmetics

x/auth/client/testutil/suite.go Outdated Show resolved Hide resolved
x/auth/client/testutil/suite.go Outdated Show resolved Hide resolved
x/auth/client/testutil/suite.go Show resolved Hide resolved
x/staking/simulation/operations.go Outdated Show resolved Hide resolved
@rach-id rach-id marked this pull request as draft September 16, 2022 11:41
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

LGTM besides the linter ofc

Comment on lines +127 to +132
// The orchestrator field is a celes1... string (i.e. sdk.AccAddress) that
// references the key that is being delegated to
string orchestrator = 12;
// This is a hex encoded 0x Ethereum public key that will be used by this
// validator on Ethereum
string eth_address = 13;
Copy link
Member

Choose a reason for hiding this comment

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

it's not possible to start a chain without these, correct?

if so, we'll need to collect this info from validators before the upgrade and include it in the genesis celestiaorg/celestia-app#608

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, we will need them.

@rach-id
Copy link
Member Author

rach-id commented Sep 19, 2022

@evan-forbes The linter is out of hand. It is failing because of a missing package:

Failure: buf.build/googleapis/googleapis:40f07f5b563941f2b20b991a7aedd53d: does not exist
make: *** [Makefile:431: proto-lint] Error 1

Probably because we're using bufbuild/buf:1.0.0-rc8 for generating proto and linting.
We can think of updating the version of buf to match upstream:

DOCKER_BUF := $(DOCKER) run --rm -v $(CURDIR):/workspace --workdir /workspace bufbuild/buf:1.7.0

But that can be handled in a separate PR. Let me know if you agree to create an issue.

PS: I did update it locally, but all the proto generated files were changed. So, it's out of scope of this PR.

For the other failing CI, it's the traditional breaking changes.

Let me know if this is alright, so we merge.

@rach-id rach-id marked this pull request as ready for review September 19, 2022 10:25
@evan-forbes
Copy link
Member

evan-forbes commented Sep 19, 2022

Yeah, let's create an issue and update the proto generation.

Then we can merge that here, and merge this

Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

👍

@rach-id
Copy link
Member Author

rach-id commented Sep 19, 2022

@evan-forbes we clicked on the same time 😃

@rach-id rach-id merged commit 482d898 into release/v0.46.x-celestia Sep 19, 2022
@evan-forbes
Copy link
Member

💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: app Changes related to the celestia-app branches enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants