-
Notifications
You must be signed in to change notification settings - Fork 34
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
use celestiaorg instead of lazyledger #28
Conversation
@@ -116,7 +116,7 @@ func InitTestnet( | |||
) error { | |||
|
|||
if chainID == "" { | |||
chainID = "chain-" + tmrand.NewRand().Str(6) | |||
chainID = "chain-" + tmrand.Str(6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just a little bit curios on why do we decide to drop NewRand()? 😶🌫️
As latest cosmos-sdk repo is still using NewRand() in their testnet file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question, check out celestiaorg/celestia-core#284
@@ -370,7 +370,7 @@ message Evidence { | |||
]; | |||
// Total voting power of the validator set in case the ABCI application does | |||
// not store historical validators. | |||
// https://github.com/lazyledger/lazyledger-core/issues/4581 | |||
// https://github.com/celestiaorg/celestia-core/issues/4581 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's better to leave tendermint here as we will be facing 404 error, if navigating with this link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch!! resolved in 21f0b78
replace github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.2-alpha.regen.4 | ||
replace ( | ||
github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.2-alpha.regen.4 | ||
github.com/golang/mock => github.com/golang/mock v1.4.4 | ||
google.golang.org/grpc => google.golang.org/grpc v1.33.2 | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had to use older versions of golang/mock
and grpc
to get tests to pass.
upstream sdk still uses v1.33.2 of grpc due to the same bugs encountered here ref Should be fixed in a future update.
As for golang/mock
, I think an updated version of the sdk will accommodate the updated version, so we can remove that replace when we update.
go.mod
Outdated
github.com/btcsuite/btcutil v1.0.2 | ||
github.com/btcsuite/btcd v0.22.0-beta | ||
github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce | ||
github.com/celestiaorg/celestia-core v0.0.1-mvp-das-lightclient.0.20210830200007-52e4383a7266 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll also update this to use the master branch as soon as we merge #522
go-lint is expected to fail due to using standard lib instead of tendermint one (see ref. |
Co-authored-by: Nguyen Nhu Viet <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #28 +/- ##
==========================================
+ Coverage 61.90% 62.89% +0.98%
==========================================
Files 609 609
Lines 35108 40392 +5284
==========================================
+ Hits 21735 25405 +3670
- Misses 11088 12664 +1576
- Partials 2285 2323 +38
|
also, the liveness test is passing for me locally, so I don't think it's worth debugging the CI atm. If it still fails after we update to the latest version of the sdk, then we can debug it then. test results
|
I think we can just update to the latest version of the sdk and see if they fixed the links. leaving a ref #27 to remind myself |
Let's hope that it will self-cure after the update 🤞 as we will be doomed to do this manual labour omt 😓 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
replaces lazyledger-core with celestia-core and updates to the latest commit of celestia-core
blocked by: #522
closes: #26