-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
refactor: reduce tendermint deps #14616
Conversation
@@ -0,0 +1,291 @@ | |||
// MODIFIED BY TENDERMINT TO EXPOSE `salt` in `GenerateFromPassword`. |
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.
Basically re-add this to the repo as it was some years ago: https://github.com/cosmos/cosmos-sdk/tree/v0.22.0/crypto/keys/bcrypt
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.
Confirmed this file is the same as https://github.com/cosmos/cosmos-sdk/blob/v0.22.0/crypto/keys/bcrypt/bcrypt.go with only benign changes.
crypto/ledger/ledger_mock.go
Outdated
@@ -1,6 +1,3 @@ | |||
//go:build ledger && test_ledger_mock |
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.
TODO: make go test ./crypto/ledger/... -tags='ledger,test_ledger_mock'
tests pass
EDIT: works
@@ -0,0 +1,35 @@ | |||
// Copyright 2011 The Go Authors. All rights reserved. |
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.
we should open an issue on how/if we can remove this file and use the canonical version
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 agree. I need to look at it more as well, as CI is not happy with this file too (there were no CI in tendermint/crypto).
I wonder if we should enhance it, or leave it as is and exclude it from linting.
EDIT: used nolint
for having CI not complain, as this is a copy of the original.
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.
Relevant: tendermint/tendermint#3027, TM removed the use of their own fork some time ago.
@odeke-em I would love if someone from your team could check this :) |
Sure, kindly paging @elias-orijtech |
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.
// Use of this source code is governed by a BSD-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package bcrypt |
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.
Confirmed this file is identical to https://cs.opensource.google/go/x/crypto/+/master:bcrypt/base64.go.
@@ -0,0 +1,291 @@ | |||
// MODIFIED BY TENDERMINT TO EXPOSE `salt` in `GenerateFromPassword`. |
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.
Confirmed this file is the same as https://github.com/cosmos/cosmos-sdk/blob/v0.22.0/crypto/keys/bcrypt/bcrypt.go with only benign changes.
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.
we should just verify the ledger changes
Just tested with my nano s and it works. |
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.
tACK with ledger nano s
(cherry picked from commit 44fbb0d) # Conflicts: # x/nft/go.mod # x/nft/go.sum
(cherry picked from commit 44fbb0d) # Conflicts: # crypto/ledger/ledger_mock.go # crypto/ledger/ledger_secp256k1.go # simapp/go.mod # simapp/go.sum # tests/go.mod # tests/go.sum # x/nft/go.mod
(cherry picked from commit 44fbb0d) # Conflicts: # crypto/ledger/ledger_mock.go # crypto/ledger/ledger_secp256k1.go # go.mod # go.sum # simapp/go.mod # simapp/go.sum # tests/go.mod # tests/go.sum # x/nft/go.mod # x/nft/go.sum
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Description
Reduces dependencies on tendermint packages due to the uncertainty about the tendermint github org:
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...
!
to the type prefix if API or client breaking changeCHANGELOG.md
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...
!
in the type prefix if API or client breaking change