Skip to content

Merge go-crypto into tendermint#1782

Merged
ebuchman merged 343 commits intotendermint:developfrom
liamsi:merge-city
Jun 21, 2018
Merged

Merge go-crypto into tendermint#1782
ebuchman merged 343 commits intotendermint:developfrom
liamsi:merge-city

Conversation

@liamsi
Copy link
Contributor

@liamsi liamsi commented Jun 21, 2018

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
    * [ ] Wrote tests doesn't apply
    * [ ] Updated CHANGELOG.md

Part of #1309
Contains a lot basically all of changes from #1721

liamsi added 5 commits June 20, 2018 17:48
makes TestListener never quit
crypto/hkdfchacha20poly1305/hkdfchachapoly_test.go:36:25:warning: should use make([]byte, 24) instead (S1019) (gosimple)
use tendermint/crypto :-)
@liamsi liamsi requested a review from cwgoes June 21, 2018 04:11
@liamsi liamsi changed the title WIP: Merge go-crypto into tendermint Merge go-crypto into tendermint Jun 21, 2018
@codecov-io
Copy link

codecov-io commented Jun 21, 2018

Codecov Report

Merging #1782 into develop will decrease coverage by 0.15%.
The diff coverage is 61.68%.

@@             Coverage Diff             @@
##           develop    #1782      +/-   ##
===========================================
- Coverage    62.59%   62.44%   -0.16%     
===========================================
  Files          126      140      +14     
  Lines        11350    11875     +525     
===========================================
+ Hits          7105     7415     +310     
- Misses        3597     3769     +172     
- Partials       648      691      +43
Impacted Files Coverage Δ
privval/socket.go 72.28% <ø> (ø) ⬆️
blockchain/wire.go 100% <ø> (ø) ⬆️
privval/wire.go 100% <ø> (ø) ⬆️
config/toml.go 53.19% <ø> (ø) ⬆️
rpc/core/pipe.go 31.7% <ø> (ø) ⬆️
p2p/key.go 70.21% <ø> (ø) ⬆️
p2p/peer.go 60.81% <ø> (ø) ⬆️
p2p/wire.go 100% <ø> (ø) ⬆️
p2p/test_util.go 56.47% <ø> (ø) ⬆️
p2p/conn/wire.go 100% <ø> (ø) ⬆️
... and 52 more

"github.com/tendermint/tmlibs/log"
)

//////////////////////////////////////////////////////////////////////////////
Copy link
Contributor

Choose a reason for hiding this comment

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

please return back

require.Equal(t, http.StatusNotFound, res.StatusCode, "should always return 404")
}

//////////////////////////////////////////////////////////////////////////////
Copy link
Contributor

Choose a reason for hiding this comment

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

please return back


// TimeFormat is used for generating the sigs
const TimeFormat = amino.RFC3339Millis
const TimeFormat = "2006-01-02T15:04:05.000Z"
Copy link
Contributor

Choose a reason for hiding this comment

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

old version const TimeFormat = amino.RFC3339Millis is correct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The version of amino this uses doesn't contain the constant anymore:
tendermint/go-amino@f81fa38

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests fail, if I do this change.

sig := pv.privKey.Sign(heartbeat.SignBytes(chainID))
sig, err := pv.privKey.Sign(heartbeat.SignBytes(chainID))
if err != nil {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

return err

sig := pv.privKey.Sign(signBytes)
sig, err := pv.privKey.Sign(signBytes)
if err != nil {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

return err

signature = locPrivKey.Sign(challenge[:])
signature, err := locPrivKey.Sign(challenge[:])
if err != nil {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a todo to modify func to return err?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Will do (and also open an issue so that doesn't get lost)

- re-add test
- add TODO
- err instead of panic where possible
@liamsi
Copy link
Contributor Author

liamsi commented Jun 21, 2018

Thanks for the review!! Addressed all your comments @melekes. PTAL

defer s.Close()

// check upgrader works
d := websocket.Dialer{}
Copy link
Contributor

Choose a reason for hiding this comment

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

what's up with formatting here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Somehow I disabled gofmt. Thanks

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Reviewed against #1721, LGTM.

@liamsi
Copy link
Contributor Author

liamsi commented Jun 21, 2018

Reviewed against #1721, LGTM.

Thank you too @cwgoes 👍

MaxPacketMsgPayloadSize int `mapstructure:"max_packet_msg_payload_size"`
// Maximum size of a message packet, in bytes
// Includes a header, which is ~13 bytes
MaxPacketMsgSize int `mapstructure:"max_packet_msg_size"`
Copy link
Contributor

Choose a reason for hiding this comment

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

breaking

plainHb := &Heartbeat{}
bz = plainHb.SignBytes("0xdeadbeef")
require.Equal(t, string(bz), `{"@chain_id":"0xdeadbeef","@type":"heartbeat","height":0,"round":0,"sequence":0,"validator_address":"","validator_index":0}`)
require.Equal(t, string(bz), `{"@chain_id":"0xdeadbeef","@type":"heartbeat","height":"0","round":"0","sequence":"0","validator_address":"","validator_index":"0"}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

so ints are strings now huh

@ebuchman ebuchman merged commit 2b5229d into tendermint:develop Jun 21, 2018
xla added a commit that referenced this pull request Jun 21, 2018
@xla xla mentioned this pull request Jun 21, 2018
2 tasks
melekes pushed a commit that referenced this pull request Jun 22, 2018
cboh4 pushed a commit to scrtlabs/tendermint that referenced this pull request Apr 7, 2025
…1782)

Otherwise, the events from app's BeginBlock won't be fired.

Closes tendermint#1468

Co-authored-by: forcodedancing <just.haha.it@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.