-
Notifications
You must be signed in to change notification settings - Fork 33
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[P2P (DUP)] fix: raintree add/remove index (#687)
## Description Fixes the index at which `sortedPeersView#Add()` and `#Remove()` operates and adds a regression test. _(Duplicate of #637, I forgot to change the base branch before merging. As a result, the changes were squashed-merged into a different branch than `main`. Re-opening here, apologies for the extra noise. 🙏)_ ## Issue Fixes #617 ## Type of change Please mark the relevant option(s): - [ ] New feature, functionality or library - [x] Bug fix - [ ] Code health or cleanup - [ ] Major breaking change - [ ] Documentation - [ ] Other <!-- add details here if it a different type of change --> ## List of changes - Add test to assert peers view addresses order using alphabet addresses (single letters) for convenience. - Fixes the sorted addresses index bug - Fixes peerstore benchmarks ## Testing _(re-done post merge w/ main)_ - [x] `make develop_test`; if any code changes were made - [x] [Docker Compose LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md); if any major functionality was changed or introduced - [x] [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any infrastructure or configuration changes were made ## Required Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added, or updated, [`godoc` format comments](https://go.dev/blog/godoc) on touched members (see: [tip.golang.org/doc/comment](https://tip.golang.org/doc/comment)) - [x] I have tested my changes using the available tooling - [x] I have updated the corresponding CHANGELOG ### If Applicable Checklist - [ ] I have updated the corresponding README(s); local and/or global - [x] I have added tests that prove my fix is effective or that my feature works - [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s) - [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s) --------- Co-authored-by: Daniel Olshansky <[email protected]> Co-authored-by: Dmitry K <[email protected]> Co-authored-by: github-actions <[email protected]>
- Loading branch information
4 people
authored
Apr 19, 2023
1 parent
33c6f96
commit f86fd55
Showing
5 changed files
with
235 additions
and
14 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,177 @@ | ||
| package types | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| cryptoPocket "github.com/pokt-network/pocket/shared/crypto" | ||
| ) | ||
|
|
||
| func TestSortedPeersView_Add_Remove(t *testing.T) { | ||
| testCases := []struct { | ||
| name string | ||
| selfAddr string | ||
| addAddrs string | ||
| removeAddrs string | ||
| expectedAddrs string | ||
| }{ | ||
| { | ||
| name: "lowest self address", | ||
| selfAddr: "A", | ||
| addAddrs: "BC", | ||
| expectedAddrs: "ABC", | ||
| }, | ||
| { | ||
| name: "highest self address", | ||
| selfAddr: "C", | ||
| addAddrs: "AB", | ||
| expectedAddrs: "CAB", | ||
| }, | ||
| { | ||
| name: "penultimate self address", | ||
| selfAddr: "W", | ||
| addAddrs: "DYZEHGI", | ||
| removeAddrs: "YE", | ||
| expectedAddrs: "WZDGHI", | ||
| }, | ||
| { | ||
| name: "middle self address", | ||
| selfAddr: "S", | ||
| addAddrs: "DTUEVGH", | ||
| removeAddrs: "E", | ||
| expectedAddrs: "STUVDGH", | ||
| }, | ||
| { | ||
| name: "discontiguous resulting addresses", | ||
| selfAddr: "O", | ||
| addAddrs: "DTURSEVGH", | ||
| removeAddrs: "EDU", | ||
| expectedAddrs: "ORSTVGH", | ||
| }, | ||
| } | ||
|
|
||
| for _, testCase := range testCases { | ||
| t.Run(testCase.name, func(t *testing.T) { | ||
| selfAddr := cryptoPocket.Address(testCase.selfAddr) | ||
| selfPeer := &NetworkPeer{Address: selfAddr} | ||
|
|
||
| pstore := make(PeerAddrMap) | ||
| err := pstore.AddPeer(selfPeer) | ||
| require.NoError(t, err) | ||
|
|
||
| view := NewSortedPeersView(selfAddr, pstore) | ||
| initialAddrs := []string{selfAddr.String()} | ||
| initialPeers := []Peer{selfPeer} | ||
|
|
||
| // assert initial state | ||
| require.ElementsMatchf(t, initialAddrs, view.sortedAddrs, "initial addresses don't match") | ||
| require.ElementsMatchf(t, initialPeers, view.sortedPeers, "initial peers don't match") | ||
|
|
||
| addrsToAdd := strings.Split(testCase.addAddrs, "") | ||
| addrsToRemove := strings.Split(testCase.removeAddrs, "") | ||
| expectedAddrs := fromCharAddrs(testCase.expectedAddrs) | ||
|
|
||
| // add peers | ||
| for _, addr := range addrsToAdd { | ||
| peer := &NetworkPeer{Address: []byte(addr)} | ||
| view.Add(peer) | ||
| t.Logf("sortedAddrs: %s", toCharAddrs(view.sortedAddrs)) | ||
| } | ||
|
|
||
| // remove peers | ||
| for _, addr := range addrsToRemove { | ||
| view.Remove([]byte(addr)) | ||
| t.Logf("sortedAddrs: %s", toCharAddrs(view.sortedAddrs)) | ||
| } | ||
|
|
||
| // assert resulting state | ||
| var expectedPeers []Peer | ||
| for _, addr := range expectedAddrs { | ||
| expectedPeers = append(expectedPeers, &NetworkPeer{Address: cryptoPocket.AddressFromString(addr)}) | ||
| } | ||
|
|
||
| actualAddrsStr := toCharAddrs(view.sortedAddrs) | ||
| require.Equal(t, testCase.expectedAddrs, actualAddrsStr, "resulting addresses don't match") | ||
| require.ElementsMatchf(t, expectedPeers, view.sortedPeers, "resulting peers don't match") | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestSortedPeersView_Remove(t *testing.T) { | ||
| t.Skip("TECHDEBT(#554): test that this method works as expected when target peer/addr is not in the list!") | ||
| } | ||
|
|
||
| func TestSortedPeersView(t *testing.T) { | ||
| testCases := []struct { | ||
| name string | ||
| selfAddr string | ||
| initialAddrs string | ||
| expectedSortOrder string | ||
| }{ | ||
| { | ||
| name: "lowest self address", | ||
| selfAddr: "A", | ||
| initialAddrs: "BDCEA", | ||
| expectedSortOrder: "ABCDE", | ||
| }, | ||
| { | ||
| name: "highest self address", | ||
| selfAddr: "E", | ||
| initialAddrs: "BDCEA", | ||
| expectedSortOrder: "EABCD", | ||
| }, | ||
| { | ||
| name: "middle self address", | ||
| selfAddr: "C", | ||
| initialAddrs: "BDCEA", | ||
| expectedSortOrder: "CDEAB", | ||
| }, | ||
| } | ||
| for _, testCase := range testCases { | ||
| t.Run(testCase.name, func(t *testing.T) { | ||
| selfAddr := cryptoPocket.Address(testCase.selfAddr) | ||
|
|
||
| pstore := make(PeerAddrMap) | ||
| var initialPeers []Peer | ||
| initialAddrs := fromCharAddrs(testCase.initialAddrs) | ||
| for _, addr := range initialAddrs { | ||
| peer := &NetworkPeer{Address: cryptoPocket.AddressFromString(addr)} | ||
| initialPeers = append(initialPeers, peer) | ||
| err := pstore.AddPeer(peer) | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| view := NewSortedPeersView(selfAddr, pstore) | ||
| require.ElementsMatchf(t, initialAddrs, view.sortedAddrs, "initial addresses don't match") | ||
| require.ElementsMatchf(t, initialPeers, view.sortedPeers, "initial peers don't match") | ||
|
|
||
| var actualPeersStr string | ||
| for _, peer := range view.sortedPeers { | ||
| actualPeersStr += string(peer.GetAddress().Bytes()) | ||
| } | ||
|
|
||
| actualAddrsStr := toCharAddrs(view.sortedAddrs) | ||
| require.Equal(t, testCase.expectedSortOrder, actualAddrsStr, "resulting addresses don't match") | ||
| require.Equal(t, testCase.expectedSortOrder, actualPeersStr, "resulting addresses don't match") | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // fromCharAddrs converts each char in charAddrs into a serialized pokt address | ||
| func fromCharAddrs(charAddrs string) (addrs []string) { | ||
| for _, ch := range strings.Split(charAddrs, "") { | ||
| addrs = append(addrs, cryptoPocket.Address(ch).String()) | ||
| } | ||
| return addrs | ||
| } | ||
|
|
||
| // toCharAddrs converts each string in addrStrs to a raw pokt address binary | ||
| // string and concatenates them for return | ||
| func toCharAddrs(addrStrs []string) (charAddrs string) { | ||
| for _, addrStr := range addrStrs { | ||
| charAddrs += string(cryptoPocket.AddressFromString(addrStr).Bytes()) | ||
| } | ||
| return charAddrs | ||
| } |