feat!: only allow chains with at least one active validator to launch #2399
Conversation
github-actions
bot
added
the
C:x/provider
insumity
force-pushed
the
insumity/at-least-one-active-validator
branch
2 times, most recently
from
151a2b1 to
bcc9dd0
Compare
insumity
force-pushed
the
insumity/at-least-one-active-validator
branch
from
bcc9dd0 to
0083f15
Compare
| TestWalletsNumber = 15 // Ensure that test accounts are used in a way that maintains the mutual independence of tests | ||
| // ValidatorCount is set to 2, so we have one active and one inactive (i.e., outside the active set) validator. | ||
| // Note that the provider has at most 1 validator (see `chain_spec_provider.go`). | ||
| ValidatorCount = 2 |
There was a problem hiding this comment.
Is this done for al the tests or only for the new one introduced? We should avoid adding complexity to all tests. If you need multiple validators for a certain test, we should just add a new configuration.
There was a problem hiding this comment.
This will affect all tests in the suite that use this specification, i.e., the entire ProviderSuite. We could create a separate suite to support running tests on a suite with multiple nodes while keeping the existing suite where the provider chain has only one node (ProviderSuite, ProviderMultiNodeSuite).
Another option is to keep it as Karolos implemented and handle the optional nodes in the suite setup, so tests that need additional nodes can simply start the extra validator(s) as the interchaintest framework supports that.
The difference is that with separate suites, we don’t need to start/stop nodes dynamically, but the provider chain will be started twice. I'm not sure what the optimal solution would be.
If you'd like, we can leave it as Karolos implemented in this PR, and I can split it or add dynamic node management in a follow-up PR. I also plan to improve how "independent" test accounts are fetched, so I can handle everything in that PR.
There was a problem hiding this comment.
Sounds good. Let's do that.
Regarding what option to choose, go with the one easier to implement.
| // Scenario 1: Inactive validators opts in, the chain does not launch. | ||
| // Scenario 2: Active validator opts in, the chain launches. | ||
| func (s *ProviderSuite) TestOptInChainCanOnlyStartIfActiveValidatorOptedIn() { | ||
| testAcc := s.Provider.TestWallets[2].FormattedAddress() |
There was a problem hiding this comment.
I'll create a separate PR so that we don't have to be aware of which index is in use during the tests, and it will be handled under the hood. For now, I would use an account that isn't used in other tests, for example, account s.Provider.TestWallets[12] (just to avoid possible an error where an invalid account number is sent if 2 tests send the tx in the same time). I'll try to fix this as soon as possible since the current solution isn't the most elegant.
Description
Closes: #2397
Testing
Introduced an interchain test.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if the change is state-machine breakingCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!the type prefix if the change is state-machine breaking