add func to set approved nodes list #1478
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1478 +/- ##
==========================================
- Coverage 55.12% 55.08% -0.04%
==========================================
Files 517 520 +3
Lines 32330 32413 +83
==========================================
+ Hits 17823 17856 +33
- Misses 12116 12160 +44
- Partials 2391 2397 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
integration/tests/epochs/suite.go
Outdated
| identities := s.net.Identities().NodeIDs() | ||
| identities = append(identities, nodes...) |
There was a problem hiding this comment.
This is making an assumption that, between calls to SetApprovedNodesScript, either the caller is arranging to add any new identities to s.net (I guess exactly how this will happen is still TBD), or they are arranging to keep track of all nodes added after startup and including them in the nodes argument for each call.
Depending on how this function is used, that's probably a fine assumption to make - but we should document it.
Another approach would just require the caller to pass in the full node ID list each time:
SetApprovedNodesScript(ctx, env, s.net.Identities().NodeIDs().Append(newNode1, newNode2))| result := s.SetApprovedNodesScript(ctx, env, info.NodeID) | ||
| require.NoError(s.T(), result.Error) |
There was a problem hiding this comment.
It would be great to include the corresponding ReadApprovedNodesScript util so that we can verify the changes as part of this PR.
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
| // get new approved nodes list and make sure new node was added correctly | ||
| approvedNodes := s.ExecuteReadApprovedNodesScript(ctx, env) | ||
|
|
||
| found := false | ||
| for _, val := range approvedNodes.(cadence.Array).Values { | ||
| if string(val.(cadence.String)) == info.NodeID.String() { | ||
| found = true | ||
| } | ||
| } | ||
|
|
||
| require.True(s.T(), found, "node id for new node not found in approved list after setting the approved list") |
There was a problem hiding this comment.
Would be great to make this a helper eg. suite.AssertInApprovedList(ctx, env, expectedNodeID).
Alternatively, you might be able to use assert.Contains(approvedNodes, info.NodeID)
|
bors merge |
This PR adds a util func to add a node to the approved nodes list. It allows epoch join tests to better mimic the process that happens when a new node is onboarded.