R4R: upgrade cosmos to 0.25 & tendermint to v0.25.1-rc0 #215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rickyyangz
commented
Oct 26, 2018
Gopkg.toml
Outdated
| @@ -57,6 +57,10 @@ | |||
| branch = "master" | |||
| name = "github.com/deathowl/go-metrics-prometheus" | |||
|
|
|||
| [prune] | |||
| go-tests = true | |||
| unused-packages = true | |||
…an work but cannot generate block
fix init does not print app message fix ante newCtx nil issue
rickyyangz
changed the title
rickyyangz
changed the title
darren-liu
requested changes
Nov 2, 2018
| @@ -59,3 +56,13 @@ | |||
| [[constraint]] | |||
| branch = "master" | |||
| name = "github.com/deathowl/go-metrics-prometheus" | |||
|
|
|||
| [[override]] | |||
| name = "golang.org/x/crypto" | |||
There was a problem hiding this comment.
let me confirm...
There was a problem hiding this comment.
cosmos-sdk overrides this version of crypto. so to use this version, we have to override here.
app/app.go
Outdated
| app.MountStoresIAVL( | ||
| common.MainStoreKey, | ||
| common.AccountStoreKey, | ||
| common.ValStoreKey, |
There was a problem hiding this comment.
It is hard to tell what ValStore is about. could we change to a clearer name?
| @@ -83,43 +53,22 @@ func NewAnteHandler(am auth.AccountMapper, orderMsgType string) sdk.AnteHandler | |||
| // collect signer accounts | |||
| // TODO: abort if there is more than one signer? | |||
| var signerAccs = make([]auth.Account, len(signerAddrs)) | |||
There was a problem hiding this comment.
this part is new change made by Luke, missed?
There was a problem hiding this comment.
his change is a workaround. so here we have used a more reasonable solution to fix the sequence issue.
| checkInvalidTx(t, anteHandler, ctx, txn, sdk.CodeInvalidPubKey) | ||
| } | ||
|
|
||
| func TestAnteHandlerGoodOrderID(t *testing.T) { |
There was a problem hiding this comment.
in the new solution, we do not check orderId in ante handler. So dropped it.
rickyyangz
commented
Nov 2, 2018
|
not much big change is required. Please try to solve the conflict and merge in next week. thanks! |
6 tasks
darren-liu
approved these changes
Nov 5, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #118
Closes: #59
Description
Upgrade cosmos and tendermint.
We switch to our own cosmos-sdk repo https://github.com/BiJie/bnc-cosmos-sdk which is forked from the official repo.
Both the new cosmos-sdk repo and BinanceChain will reference to same version of tendermint which is
v0.25.1-rc0 currently.
Rationale
Too many files copied from the original cosmos-sdk since we have lot of customized features.
If not forking, more and more files need copy which would make the future upgrade harder and harder.
Example
Changes
Notable changes:
Preflight checks
make build)make test)make integration_test)Already reviewed by
@ackratos @guagualvcha
Related issues