Skip to content

Commit

Permalink
Secret conn remove pool buffer, align with upstream (#123)
Browse files Browse the repository at this point in the history
* Secret conn remove pool buffer, align with upstream

* add changelog
  • Loading branch information
ValarDragon authored Jul 3, 2024
1 parent a53d2dd commit d941920
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[p2p/conn]` Remove the usage of a synchronous pool of buffers in secret connection, storing instead the buffer in the connection struct. This reduces the synchronization primitive usage, speeding up the code.
([\#3403](https://github.com/cometbft/cometbft/issues/3403))
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# CHANGELOG

## Unreleased (I think)

* [#123](https://github.com/osmosis-labs/cometbft/pull/123) perf(p2p/conn): Remove unneeded global pool buffers in secret connection #3403
* perf(p2p): Delete expensive debug log already slated for deletion #3412
* perf(p2p): Reduce the p2p metrics overhead. #3411
* commit f663bd35153b0b366c1e1e6b41e7f2dcff7963fd : one more debug log deletion
* [#120](https://github.com/osmosis-labs/cometbft/pull/120) perf(consensus): Use TrySend for hasVote/HasBlockPart messages #3407


## v0.37.4-v25-osmo-11

* [#117](https://github.com/osmosis-labs/cometbft/pull/117) fix(mempool)!: stop accepting TXs in the mempool if we can't keep up
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ require (
github.com/gorilla/websocket v1.5.0
github.com/informalsystems/tm-load-test v1.3.0
github.com/lib/pq v1.10.7
github.com/libp2p/go-buffer-pool v0.1.0
github.com/minio/highwayhash v1.0.2
github.com/ory/dockertest v3.3.5+incompatible
github.com/pkg/errors v0.9.1
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,6 @@ github.com/leonklingele/grouper v1.1.0/go.mod h1:uk3I3uDfi9B6PeUjsCKi6ndcf63Uy7s
github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
github.com/lib/pq v1.10.7 h1:p7ZhMD+KsSRozJr34udlUrhboJwWAgCg34+/ZZNvZZw=
github.com/lib/pq v1.10.7/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/libp2p/go-buffer-pool v0.1.0 h1:oK4mSFcQz7cTQIfqbe4MIj9gLW+mnanjyFtc6cdF0Y8=
github.com/libp2p/go-buffer-pool v0.1.0/go.mod h1:N+vh8gMqimBzdKkSMVuydVDq+UV5QTWy5HSiZacSbPg=
github.com/lufeee/execinquery v1.2.1 h1:hf0Ems4SHcUGBxpGN7Jz78z1ppVkP/837ZlETPCEtOM=
github.com/lufeee/execinquery v1.2.1/go.mod h1:EC7DrEKView09ocscGHC+apXMIaorh4xqSxS/dy8SbM=
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
Expand Down
19 changes: 11 additions & 8 deletions p2p/conn/evil_secret_connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,17 @@ func (c *evilConn) signChallenge() []byte {

b := &buffer{}
c.secretConn = &SecretConnection{
underlyingConn: b,
connReader: b,
connWriter: bufio.NewWriterSize(b, 65536),
recvBuffer: nil,
recvNonce: new([aeadNonceSize]byte),
sendNonce: new([aeadNonceSize]byte),
recvAead: recvAead,
sendAead: sendAead,
conn: b,
connWriter: bufio.NewWriter(b),
recvBuffer: nil,
recvNonce: new([aeadNonceSize]byte),
sendNonce: new([aeadNonceSize]byte),
recvAead: recvAead,
sendAead: sendAead,
recvFrame: make([]byte, totalFrameSize),
recvSealedFrame: make([]byte, totalFrameSize+aeadSizeOverhead),
sendFrame: make([]byte, totalFrameSize),
sendSealedFrame: make([]byte, totalFrameSize+aeadSizeOverhead),
}
c.buffer = b

Expand Down
68 changes: 33 additions & 35 deletions p2p/conn/secret_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"time"

gogotypes "github.com/cosmos/gogoproto/types"
pool "github.com/libp2p/go-buffer-pool"
"github.com/oasisprotocol/curve25519-voi/primitives/merlin"
"golang.org/x/crypto/chacha20poly1305"
"golang.org/x/crypto/curve25519"
Expand Down Expand Up @@ -72,9 +71,8 @@ type SecretConnection struct {

remPubKey crypto.PubKey

underlyingConn io.ReadWriteCloser
connWriter *bufio.Writer
connReader io.Reader
conn io.ReadWriteCloser
connWriter *bufio.Writer

// net.Conn must be thread safe:
// https://golang.org/pkg/net/#Conn.
Expand All @@ -83,12 +81,16 @@ type SecretConnection struct {
// are independent, so we can use two mtxs.
// All .Read are covered by recvMtx,
// all .Write are covered by sendMtx.
recvMtx cmtsync.Mutex
recvBuffer []byte
recvNonce *[aeadNonceSize]byte

sendMtx cmtsync.Mutex
sendNonce *[aeadNonceSize]byte
recvMtx cmtsync.Mutex
recvBuffer []byte
recvNonce *[aeadNonceSize]byte
recvFrame []byte
recvSealedFrame []byte

sendMtx cmtsync.Mutex
sendNonce *[aeadNonceSize]byte
sendFrame []byte
sendSealedFrame []byte
}

// MakeSecretConnection performs handshake and returns a new authenticated
Expand Down Expand Up @@ -151,14 +153,17 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*
}

sc := &SecretConnection{
underlyingConn: conn,
connWriter: bufio.NewWriterSize(conn, defaultWriteBufferSize),
connReader: conn,
recvBuffer: nil,
recvNonce: new([aeadNonceSize]byte),
sendNonce: new([aeadNonceSize]byte),
recvAead: recvAead,
sendAead: sendAead,
conn: conn,
connWriter: bufio.NewWriterSize(conn, defaultWriteBufferSize),
recvBuffer: nil,
recvNonce: new([aeadNonceSize]byte),
sendNonce: new([aeadNonceSize]byte),
recvAead: recvAead,
sendAead: sendAead,
recvFrame: make([]byte, totalFrameSize),
recvSealedFrame: make([]byte, aeadSizeOverhead+totalFrameSize),
sendFrame: make([]byte, totalFrameSize),
sendSealedFrame: make([]byte, aeadSizeOverhead+totalFrameSize),
}

// Sign the challenge bytes for authentication.
Expand Down Expand Up @@ -196,15 +201,10 @@ func (sc *SecretConnection) RemotePubKey() crypto.PubKey {
func (sc *SecretConnection) Write(data []byte) (n int, err error) {
sc.sendMtx.Lock()
defer sc.sendMtx.Unlock()
sealedFrame, frame := sc.sendSealedFrame, sc.sendFrame

for 0 < len(data) {
if err := func() error {
var sealedFrame = pool.Get(aeadSizeOverhead + totalFrameSize)
var frame = pool.Get(totalFrameSize)
defer func() {
pool.Put(sealedFrame)
pool.Put(frame)
}()
var chunk []byte
if dataMaxSize < len(data) {
chunk = data[:dataMaxSize]
Expand Down Expand Up @@ -249,17 +249,15 @@ func (sc *SecretConnection) Read(data []byte) (n int, err error) {
}

// read off the conn
var sealedFrame = pool.Get(aeadSizeOverhead + totalFrameSize)
defer pool.Put(sealedFrame)
_, err = io.ReadFull(sc.connReader, sealedFrame)
sealedFrame := sc.recvSealedFrame
_, err = io.ReadFull(sc.conn, sealedFrame)
if err != nil {
return
}

// decrypt the frame.
// reads and updates the sc.recvNonce
var frame = pool.Get(totalFrameSize)
defer pool.Put(frame)
frame := sc.recvFrame
_, err = sc.recvAead.Open(frame[:0], sc.recvNonce[:], sealedFrame, nil)
if err != nil {
return n, fmt.Errorf("failed to decrypt SecretConnection: %w", err)
Expand All @@ -283,19 +281,19 @@ func (sc *SecretConnection) Read(data []byte) (n int, err error) {
}

// Implements net.Conn
func (sc *SecretConnection) Close() error { return sc.underlyingConn.Close() }
func (sc *SecretConnection) LocalAddr() net.Addr { return sc.underlyingConn.(net.Conn).LocalAddr() }
func (sc *SecretConnection) RemoteAddr() net.Addr { return sc.underlyingConn.(net.Conn).RemoteAddr() }
func (sc *SecretConnection) Close() error { return sc.conn.Close() }
func (sc *SecretConnection) LocalAddr() net.Addr { return sc.conn.(net.Conn).LocalAddr() }
func (sc *SecretConnection) RemoteAddr() net.Addr { return sc.conn.(net.Conn).RemoteAddr() }
func (sc *SecretConnection) SetDeadline(t time.Time) error {
return sc.underlyingConn.(net.Conn).SetDeadline(t)
return sc.conn.(net.Conn).SetDeadline(t)
}

func (sc *SecretConnection) SetReadDeadline(t time.Time) error {
return sc.underlyingConn.(net.Conn).SetReadDeadline(t)
return sc.conn.(net.Conn).SetReadDeadline(t)
}

func (sc *SecretConnection) SetWriteDeadline(t time.Time) error {
return sc.underlyingConn.(net.Conn).SetWriteDeadline(t)
return sc.conn.(net.Conn).SetWriteDeadline(t)
}

func genEphKeys() (ephPub, ephPriv *[32]byte) {
Expand Down

0 comments on commit d941920

Please sign in to comment.