algorand / algorand/go-algorand
Simplify our approach to referencing ConsensusVersion in Tests
- Dominant language
- Go
- Stars
- 1.4k
- Forks
- 537
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 17
Description
## Problem
Throughout go-algorand tests, there are varying references to `config.Consensus[protocol.ConsensusFuture]`, `config.Consensus[protocol.ConsensusCurrentVersion]`, or `config.Consensus[]`. Explicitly NOT referring to `LogicVersion` in this issue, as the rules around TEAL versions are quite different.
There are a few considerations:
1. While we are exercising new functionality, we'll reference `protocol.ConsensusFuture`. When it is actually time to do a consensus upgrade, we attempt to update all of these references to the minimum version that the feature should start working (some set it to `ConsensusCurrentVersion`). We often miss updating all of these places during release prep and end up leaving the feature being tested as future. When we do remember to update, we do not consistently set the version to the minimum from which that functionality should start operating.
2. The scenarios we primarily may want to exercise include current consensus version, prior consensus version (the `current` version may not have been adopted for a period of time, hence ensuring prior one does not activate new functionality), and `future`. Today, most of our tests only exercise one of these scenarios based on what is hard-coded.
## Solution Option 1
For tests, we should consider parameterizing so that we can run against multiple consensus versions. Can then set up runners that execute:
* "Current Consensus Version"
* Future Consensus Version
* (TBD) Prior Consensus Version
If we go with parameterization, we would no longer have to update versions -and- can test behavior under multiple relevant consensus versions.
We'll need to talk tradeoffs between amount of time/resources to run these (likely turning prior consensus version off once new consensus takes effect).
## Solution Option 2
If we decided against parameterizing, at minimum we should consider a resolver utility function that identifies the minimum version that a particular parameter is activated/set to a non-default value. This provides us a consistent approach throughout tests, and avoids having to go through code when prepping a consensus upgrade making protocol reference changes.
## Dependencies
Worth discussing impact on multiple workstreams since doing this impacts most development in go-algorand.
## Urgency
Low - we can continue operating as-is, though we carry some cognitive overhead.
Contributor guide
Assessment
This issue has not been assessed yet.