Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Staking v4 - Integration tests with custom scenarios #4045

Conversation

mariusmihaic
Copy link
Contributor

@mariusmihaic mariusmihaic commented May 3, 2022

Bug fixes

  • Fixed bug in testing helper func AddValidatorData

Refactor

  • Moved GetAllNodeKeys from systemSCs.go to a new file to be used in tests as well
  • Moved ProcessSCOutputAccounts from TestNodeProcessor to be used in staking v4 tests
  • Added baseTestMetaProcessor.go which handles creation of a TestMetaProcessor regardless of NewTestMetaProcessor/NewTestMetaProcessorWithCustomNodes. Most of the functionality in this file is taken from testMetaProcessor.go

New tests

  • Added StakingToPeer to handle staking new nodes after staking v4 epoch, which should save new peer accounts in auction
  • TestStakingV4_UnStakeNodesWithNotEnoughFunds which tests how nodes are unstaked at the end of the epoch in case they don't have enough funds. Before staking v4(including staking v4 init epoch), nodes should be unstaked based on prority: queue -> waiting -> eligible. After staking v4, priority should be auction -> waiting -> eligible
  • TestStakingV4_StakeNewNodes tests functionality of staking new nodes after staking v4(they should be sent to auction)

@mariusmihaic mariusmihaic changed the title FEAT: Add initial placeholder file Staking v4 - Integration tests with custom scenarios May 3, 2022
@mariusmihaic mariusmihaic changed the base branch from EN-11957-move-unjailed-new-nodes-to-auction to fix-waiting-list-tests May 3, 2022 13:37
@mariusmihaic mariusmihaic self-assigned this May 4, 2022
Base automatically changed from fix-waiting-list-tests to feat/liquid-staking May 4, 2022 13:03
@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #4045 (4635333) into feat/liquid-staking (1464814) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head 4635333 differs from pull request most recent head 697aea6. Consider uploading reports for the commit 697aea6 to get more accurate results

@@                   Coverage Diff                   @@
##           feat/liquid-staking    #4045      +/-   ##
=======================================================
- Coverage                75.38%   75.38%   -0.01%     
=======================================================
  Files                      622      622              
  Lines                    83275    83275              
=======================================================
- Hits                     62779    62778       -1     
- Misses                   15773    15774       +1     
  Partials                  4723     4723              
Impacted Files Coverage Δ
epochStart/metachain/systemSCs.go 67.38% <0.00%> (ø)
p2p/libp2p/netMessenger.go 74.83% <0.00%> (-0.14%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3bbea0...697aea6. Read the comment docs.

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2022

Codecov Report

Merging #4045 (68a602a) into feat/liquid-staking (1464814) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@                   Coverage Diff                   @@
##           feat/liquid-staking    #4045      +/-   ##
=======================================================
- Coverage                75.38%   75.38%   -0.01%     
=======================================================
  Files                      622      622              
  Lines                    83275    83275              
=======================================================
- Hits                     62779    62777       -2     
- Misses                   15773    15774       +1     
- Partials                  4723     4724       +1     
Impacted Files Coverage Δ
epochStart/metachain/systemSCs.go 67.38% <0.00%> (ø)
p2p/libp2p/netMessenger.go 74.70% <0.00%> (-0.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3bbea0...68a602a. Read the comment docs.

@mariusmihaic mariusmihaic marked this pull request as ready for review May 12, 2022 10:10
@bogdan-rosianu bogdan-rosianu self-requested a review May 12, 2022 10:44
) *TestMetaProcessor {
gasScheduleNotifier := createGasScheduleNotifier()
blockChainHook := createBlockChainHook(
dataComponents, coreComponents,
Copy link
Contributor

Choose a reason for hiding this comment

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

coreComponents on new line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, missed it;
Added new line

@@ -65,7 +73,37 @@ func requireMapDoesNotContain(t *testing.T, m map[uint32][][]byte, s [][]byte) {
}
}

// TODO: Staking v4: more tests to check exactly which nodes have been selected/unselected from previous nodes config auction
func remove(s [][]byte, elem []byte) [][]byte {
Copy link
Contributor

Choose a reason for hiding this comment

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

please rename and use better names for variables. a better name would be removeItemFromSliceWithoutKeepingOrder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this name is a bit too lengthy and maybe not worth describing its behavior for a test. Added a comm to highlight its behavior. Also, renamed s -> slice

@@ -320,7 +320,7 @@ func calcNormRand(randomness []byte, expectedLen int) []byte {
randLen := len(rand)

if expectedLen > randLen {
repeatedCt := expectedLen/randLen + 1
repeatedCt := expectedLen/randLen + 1 // todo: fix possible div by 0
Copy link
Contributor

Choose a reason for hiding this comment

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

fix it now :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Left a //todo there so I can fix it in the next PR in which I will create a separate component for auction selection.
This is going to be fixed in the next PR, along with the new subcomp, so we don't "clutter" this PR with anymore changes.

@multiversx multiversx deleted a comment from mariusmihaic May 12, 2022
@mariusmihaic mariusmihaic merged commit 965efba into feat/liquid-staking May 23, 2022
@mariusmihaic mariusmihaic deleted the EN-11959-staking-v4-integration-tests-custom-scenarios branch May 23, 2022 07:39
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.

None yet

4 participants