Skip to content

remove the map usage#3296

Open
juniuszhou wants to merge 23 commits intofuture/peggy2from
junius-3257
Open

remove the map usage#3296
juniuszhou wants to merge 23 commits intofuture/peggy2from
junius-3257

Conversation

@juniuszhou
Copy link
Copy Markdown
Contributor

No description provided.

@juniuszhou juniuszhou self-assigned this Oct 3, 2022
@juniuszhou juniuszhou added the Peggy Team Peggy team task label Oct 3, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 4, 2022

Codecov Report

Merging #3296 (1821e40) into future/peggy2 (efe6269) will decrease coverage by 0.09%.
The diff coverage is 85.50%.

❗ Current head 1821e40 differs from pull request most recent head 6045e82. Consider uploading reports for the commit 6045e82 to get more accurate results

Impacted file tree graph

@@                Coverage Diff                @@
##           future/peggy2    #3296      +/-   ##
=================================================
- Coverage          48.27%   48.18%   -0.10%     
=================================================
  Files                169      171       +2     
  Lines              14940    15102     +162     
=================================================
+ Hits                7213     7277      +64     
- Misses              7307     7389      +82     
- Partials             420      436      +16     
Impacted Files Coverage Δ
cmd/ebrelayer/txs/relayToEthereum.go 0.00% <0.00%> (ø)
x/instrumentation/test_instrumentation.go 50.00% <ø> (ø)
cmd/sifnoded/cmd/gentx.go 61.90% <61.29%> (+1.24%) ⬆️
x/tokenregistry/keeper/keeper.go 89.60% <64.28%> (-3.21%) ⬇️
x/oracle/keeper/keeper.go 56.81% <68.00%> (+2.40%) ⬆️
x/oracle/types/globalSequenceKey.go 81.81% <81.81%> (ø)
x/oracle/keeper/prophecy.go 84.29% <90.90%> (-0.24%) ⬇️
x/oracle/types/validatorWhiteList.go 90.90% <90.90%> (ø)
x/oracle/keeper/validatorWhiteList.go 87.50% <92.85%> (+1.34%) ⬆️
x/oracle/keeper/witnessLockBurnSequence.go 92.85% <92.85%> (ø)
... and 31 more

@juniuszhou juniuszhou marked this pull request as ready for review October 11, 2022 02:45
@juniuszhou juniuszhou requested review from Brando753, banshee and smartyalgo and removed request for Brando753 October 11, 2022 02:45
expectedValidatorAddress, err := sdk.ValAddressFromBech32(expectedValidatorBech32)
assert.NoError(t, err)
found := false
for _, value := range validators.ValidatorPower {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use the new slice functions to find addresses rather then loop through https://pkg.go.dev/golang.org/x/exp/slices#Index to minimize repeating code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

tried to use the Index in slices package, prefer not using it.

  1. we need create a new slice for all validator addresses
  2. validator address is not comparable

Comment thread cmd/sifnoded/cmd/gentx.go
config.SetRoot(clientCtx.HomeDir)

networkDescriptor, err := strconv.ParseUint(args[0], 10, 32)
networkDescriptorNum, err := strconv.ParseUint(args[0], 10, 32)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For lines 35~44 replace with call to func parseNetworkDescriptor(networkDescriptorStr string) (oracletypes.NetworkDescriptor, error) in x/ethbridge/client/cli/tx.go. May need to move that function to a more central location.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I implemented it in other PR.

Comment thread cmd/sifnoded/cmd/gentx.go Outdated
validatorWhitelist := oracletypes.ValidatorWhiteList{}

// find and remove according to network descriptor
for index := 0; index < len(oracleGenState.ValidatorWhitelist); index++ {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Again here lets use the slices packages https://pkg.go.dev/golang.org/x/exp/slices#Delete or something similar

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is not to remove a networkDescriptor from a slice of networkDescriptors. So we still need find out it at first, then remove an item from ValidatorWhitelist. we could use filter/collect if go lang support the functional programming well.

map<uint32, uint32> consensus_needed = 4;
map<uint32, CrossChainFeeConfig> cross_chain_fee = 5;
map<string, uint64> witness_lock_burn_sequence = 6;
map<string, ProphecyInfo> prophecy_info = 7;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets make a message for NetworkConfigData which contains the address_whitelist, consensus_needed, and the cross_chain_fee so we don't have to look them up individually. And these become a repeated struct. This struct also needs to include the NetworkDescriptor number so that indexing can be done properly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will add NetworkConfigData

Comment thread proto/sifnode/oracle/v1/types.proto Outdated
uint32 voting_power = 2;
}

message GenesisValidatorWhiteList {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These three should be joined into one message type

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will combine these three items.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Peggy Team Peggy team task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants