From 7fe69ba2978a8efd9c3bc7cb1606ae8677c6b3f2 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 14 Jun 2023 21:28:48 +0800 Subject: [PATCH 1/3] fix: avoid recursive call after rename to (*PeerState).MarshalJSON (#865) * avoid recursive call after rename to (*PeerState).MarshalJSON * add test * add change doc * explain for nolint * fix lint * fix golangci-lint to v1.52.2 * fix golangci-lint to v1.52.2 * Revert "fix golangci-lint to v1.52.2" This reverts commit 598a9ef4c86fc29cf038251676c33a222217826c. * Revert "fix golangci-lint to v1.52.2" This reverts commit a8aad121e27382813e95b1911b1b560c62e1c7c3. * Reintroduced `cmtjson` * Avoid copying Mutex * Avoid copying Mutex -- 2nd try, more succint * Update .changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md * Update consensus/reactor_test.go --------- Co-authored-by: Sergio Mena (cherry picked from commit f6ea09171a2bf9f695f59b65f5c51e4a8c168015) # Conflicts: # consensus/reactor_test.go --- .../865-fix-peerstate-marshaljson.md | 2 + consensus/reactor.go | 3 +- consensus/reactor_test.go | 54 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 .changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md diff --git a/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md b/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md new file mode 100644 index 00000000000..318bda315c5 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md @@ -0,0 +1,2 @@ +- `[consensus]` Avoid recursive call after rename to (*PeerState).MarshalJSON + ([\#863](https://github.com/cometbft/cometbft/pull/863)) diff --git a/consensus/reactor.go b/consensus/reactor.go index aa93f5ac9d3..7dda1c73da4 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1089,7 +1089,8 @@ func (ps *PeerState) MarshalJSON() ([]byte, error) { ps.mtx.Lock() defer ps.mtx.Unlock() - return cmtjson.Marshal(ps) + type jsonPeerState PeerState + return cmtjson.Marshal((*jsonPeerState)(ps)) } // GetHeight returns an atomic snapshot of the PeerRoundState's height diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index dbdafde35a8..bdc36927018 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -18,6 +18,7 @@ import ( dbm "github.com/cometbft/cometbft-db" +<<<<<<< HEAD abcicli "github.com/tendermint/tendermint/abci/client" "github.com/tendermint/tendermint/abci/example/kvstore" abci "github.com/tendermint/tendermint/abci/types" @@ -40,6 +41,30 @@ import ( statemocks "github.com/tendermint/tendermint/state/mocks" "github.com/tendermint/tendermint/store" "github.com/tendermint/tendermint/types" +======= + abcicli "github.com/cometbft/cometbft/abci/client" + "github.com/cometbft/cometbft/abci/example/kvstore" + abci "github.com/cometbft/cometbft/abci/types" + cfg "github.com/cometbft/cometbft/config" + cstypes "github.com/cometbft/cometbft/consensus/types" + cryptoenc "github.com/cometbft/cometbft/crypto/encoding" + "github.com/cometbft/cometbft/crypto/tmhash" + "github.com/cometbft/cometbft/libs/bits" + "github.com/cometbft/cometbft/libs/bytes" + "github.com/cometbft/cometbft/libs/json" + "github.com/cometbft/cometbft/libs/log" + cmtsync "github.com/cometbft/cometbft/libs/sync" + mempl "github.com/cometbft/cometbft/mempool" + "github.com/cometbft/cometbft/p2p" + p2pmock "github.com/cometbft/cometbft/p2p/mock" + cmtcons "github.com/cometbft/cometbft/proto/tendermint/consensus" + cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + "github.com/cometbft/cometbft/proxy" + sm "github.com/cometbft/cometbft/state" + statemocks "github.com/cometbft/cometbft/state/mocks" + "github.com/cometbft/cometbft/store" + "github.com/cometbft/cometbft/types" +>>>>>>> f6ea09171 (fix: avoid recursive call after rename to (*PeerState).MarshalJSON (#865)) ) //---------------------------------------------- @@ -1032,3 +1057,32 @@ func TestVoteSetBitsMessageValidateBasic(t *testing.T) { }) } } + +func TestMarshalJSONPeerState(t *testing.T) { + ps := NewPeerState(nil) + data, err := json.Marshal(ps) + require.NoError(t, err) + require.JSONEq(t, `{ + "round_state":{ + "height": "0", + "round": -1, + "step": 0, + "start_time": "0001-01-01T00:00:00Z", + "proposal": false, + "proposal_block_part_set_header": + {"total":0, "hash":""}, + "proposal_block_parts": null, + "proposal_pol_round": -1, + "proposal_pol": null, + "prevotes": null, + "precommits": null, + "last_commit_round": -1, + "last_commit": null, + "catchup_commit_round": -1, + "catchup_commit": null + }, + "stats":{ + "votes":"0", + "block_parts":"0"} + }`, string(data)) +} From 8bc9678fa58d06b2c9eecf99d5d32c5832b2b100 Mon Sep 17 00:00:00 2001 From: Sergio Mena Date: Wed, 14 Jun 2023 15:43:56 +0200 Subject: [PATCH 2/3] Revert "fix: avoid recursive call after rename to (*PeerState).MarshalJSON (#865)" --- .../865-fix-peerstate-marshaljson.md | 2 - consensus/reactor.go | 3 +- consensus/reactor_test.go | 54 ------------------- 3 files changed, 1 insertion(+), 58 deletions(-) delete mode 100644 .changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md diff --git a/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md b/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md deleted file mode 100644 index 318bda315c5..00000000000 --- a/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md +++ /dev/null @@ -1,2 +0,0 @@ -- `[consensus]` Avoid recursive call after rename to (*PeerState).MarshalJSON - ([\#863](https://github.com/cometbft/cometbft/pull/863)) diff --git a/consensus/reactor.go b/consensus/reactor.go index 7dda1c73da4..aa93f5ac9d3 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1089,8 +1089,7 @@ func (ps *PeerState) MarshalJSON() ([]byte, error) { ps.mtx.Lock() defer ps.mtx.Unlock() - type jsonPeerState PeerState - return cmtjson.Marshal((*jsonPeerState)(ps)) + return cmtjson.Marshal(ps) } // GetHeight returns an atomic snapshot of the PeerRoundState's height diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index bdc36927018..dbdafde35a8 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -18,7 +18,6 @@ import ( dbm "github.com/cometbft/cometbft-db" -<<<<<<< HEAD abcicli "github.com/tendermint/tendermint/abci/client" "github.com/tendermint/tendermint/abci/example/kvstore" abci "github.com/tendermint/tendermint/abci/types" @@ -41,30 +40,6 @@ import ( statemocks "github.com/tendermint/tendermint/state/mocks" "github.com/tendermint/tendermint/store" "github.com/tendermint/tendermint/types" -======= - abcicli "github.com/cometbft/cometbft/abci/client" - "github.com/cometbft/cometbft/abci/example/kvstore" - abci "github.com/cometbft/cometbft/abci/types" - cfg "github.com/cometbft/cometbft/config" - cstypes "github.com/cometbft/cometbft/consensus/types" - cryptoenc "github.com/cometbft/cometbft/crypto/encoding" - "github.com/cometbft/cometbft/crypto/tmhash" - "github.com/cometbft/cometbft/libs/bits" - "github.com/cometbft/cometbft/libs/bytes" - "github.com/cometbft/cometbft/libs/json" - "github.com/cometbft/cometbft/libs/log" - cmtsync "github.com/cometbft/cometbft/libs/sync" - mempl "github.com/cometbft/cometbft/mempool" - "github.com/cometbft/cometbft/p2p" - p2pmock "github.com/cometbft/cometbft/p2p/mock" - cmtcons "github.com/cometbft/cometbft/proto/tendermint/consensus" - cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" - "github.com/cometbft/cometbft/proxy" - sm "github.com/cometbft/cometbft/state" - statemocks "github.com/cometbft/cometbft/state/mocks" - "github.com/cometbft/cometbft/store" - "github.com/cometbft/cometbft/types" ->>>>>>> f6ea09171 (fix: avoid recursive call after rename to (*PeerState).MarshalJSON (#865)) ) //---------------------------------------------- @@ -1057,32 +1032,3 @@ func TestVoteSetBitsMessageValidateBasic(t *testing.T) { }) } } - -func TestMarshalJSONPeerState(t *testing.T) { - ps := NewPeerState(nil) - data, err := json.Marshal(ps) - require.NoError(t, err) - require.JSONEq(t, `{ - "round_state":{ - "height": "0", - "round": -1, - "step": 0, - "start_time": "0001-01-01T00:00:00Z", - "proposal": false, - "proposal_block_part_set_header": - {"total":0, "hash":""}, - "proposal_block_parts": null, - "proposal_pol_round": -1, - "proposal_pol": null, - "prevotes": null, - "precommits": null, - "last_commit_round": -1, - "last_commit": null, - "catchup_commit_round": -1, - "catchup_commit": null - }, - "stats":{ - "votes":"0", - "block_parts":"0"} - }`, string(data)) -} From 0e78f7e4137506d12038fc5a14a0b43684971985 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 14 Jun 2023 21:28:48 +0800 Subject: [PATCH 3/3] fix: avoid recursive call after rename to (*PeerState).MarshalJSON (#865) * avoid recursive call after rename to (*PeerState).MarshalJSON * add test * add change doc * explain for nolint * fix lint * fix golangci-lint to v1.52.2 * fix golangci-lint to v1.52.2 * Revert "fix golangci-lint to v1.52.2" This reverts commit 598a9ef4c86fc29cf038251676c33a222217826c. * Revert "fix golangci-lint to v1.52.2" This reverts commit a8aad121e27382813e95b1911b1b560c62e1c7c3. * Reintroduced `cmtjson` * Avoid copying Mutex * Avoid copying Mutex -- 2nd try, more succint * Update .changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md * Update consensus/reactor_test.go --------- Co-authored-by: Sergio Mena --- .../865-fix-peerstate-marshaljson.md | 2 ++ consensus/reactor.go | 3 +- consensus/reactor_test.go | 30 +++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 .changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md diff --git a/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md b/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md new file mode 100644 index 00000000000..318bda315c5 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/865-fix-peerstate-marshaljson.md @@ -0,0 +1,2 @@ +- `[consensus]` Avoid recursive call after rename to (*PeerState).MarshalJSON + ([\#863](https://github.com/cometbft/cometbft/pull/863)) diff --git a/consensus/reactor.go b/consensus/reactor.go index aa93f5ac9d3..7dda1c73da4 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1089,7 +1089,8 @@ func (ps *PeerState) MarshalJSON() ([]byte, error) { ps.mtx.Lock() defer ps.mtx.Unlock() - return cmtjson.Marshal(ps) + type jsonPeerState PeerState + return cmtjson.Marshal((*jsonPeerState)(ps)) } // GetHeight returns an atomic snapshot of the PeerRoundState's height diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index dbdafde35a8..d5dae78dc02 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -2,6 +2,7 @@ package consensus import ( "context" + "encoding/json" "fmt" "os" "path" @@ -1032,3 +1033,32 @@ func TestVoteSetBitsMessageValidateBasic(t *testing.T) { }) } } + +func TestMarshalJSONPeerState(t *testing.T) { + ps := NewPeerState(nil) + data, err := json.Marshal(ps) + require.NoError(t, err) + require.JSONEq(t, `{ + "round_state":{ + "height": "0", + "round": -1, + "step": 0, + "start_time": "0001-01-01T00:00:00Z", + "proposal": false, + "proposal_block_part_set_header": + {"total":0, "hash":""}, + "proposal_block_parts": null, + "proposal_pol_round": -1, + "proposal_pol": null, + "prevotes": null, + "precommits": null, + "last_commit_round": -1, + "last_commit": null, + "catchup_commit_round": -1, + "catchup_commit": null + }, + "stats":{ + "votes":"0", + "block_parts":"0"} + }`, string(data)) +}