Skip to content

Commit

Permalink
blockchain: Make zero val threshold tuple invalid.
Browse files Browse the repository at this point in the history
This makes the zero value for a threshold state tuple act as the invalid
state and choice instead of using magic sentinel values.

In practice, callers should be checking the error of any methods before
using any other returned values, however, the code typically tries to
adopt a defense in depth model where it makes sure unusable/invalid
values are also returned in error cases to help make it obvious when a
caller incorrectly uses the returned value without checking the error.

It accomplishes this by reording the threshold states so that the
invalid state is 0 and converting the choice field to a pointer to the
relevant choice as opposed to a choice index.  That way the zero value
for the overall type is the invalid state and nil the choice, exactly as
expected.

In addition to being more ergonomic and inline with typical Go code, it
has a few additional benefits:

* Makes the type harder to misuse
* Simplifies identification of a specific resulting choice for votes
  that have multiple affirmative choices since the choice can be checked
  by its ID instead of a tightly coupled array index
* Provides an easy method to distinguish between a vote that failed due
  to a majority vote and one that failed due to expiring before a
  majority result was achieved
  • Loading branch information
davecgh committed Mar 14, 2023
1 parent e22c50a commit 59e1d24
Show file tree
Hide file tree
Showing 6 changed files with 336 additions and 335 deletions.
4 changes: 1 addition & 3 deletions internal/blockchain/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -2121,12 +2121,10 @@ func determineForcedThresholdState(deployment *chaincfg.ConsensusDeployment) (*T
// Attempt to find the choice with the ID that matches the forced choice
// specified in the deployment chain params and ensure it exists.
var forcedChoice *chaincfg.Choice
var forcedChoiceIdx uint32
for choiceIdx := range deployment.Vote.Choices {
choice := &deployment.Vote.Choices[choiceIdx]
if choice.Id == forcedChoiceID {
forcedChoice = choice
forcedChoiceIdx = uint32(choiceIdx)
break
}
}
Expand All @@ -2151,7 +2149,7 @@ func determineForcedThresholdState(deployment *chaincfg.ConsensusDeployment) (*T
if forcedChoice.IsNo {
state = ThresholdFailed
}
tuple := newThresholdState(state, forcedChoiceIdx)
tuple := newThresholdState(state, forcedChoice)
return &tuple, nil
}

Expand Down
15 changes: 11 additions & 4 deletions internal/blockchain/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ func (g *chaingenHarness) TestThresholdState(id string, state ThresholdState) {
// TestThresholdStateChoice queries the threshold state from the current tip
// block associated with the harness generator and expects the returned state
// and choice to match the provided value.
func (g *chaingenHarness) TestThresholdStateChoice(id string, state ThresholdState, choice uint32) {
func (g *chaingenHarness) TestThresholdStateChoice(id string, state ThresholdState, choice *chaincfg.Choice) {
g.t.Helper()

tipHash := g.Tip().BlockHash()
Expand All @@ -1073,10 +1073,17 @@ func (g *chaingenHarness) TestThresholdStateChoice(id string, state ThresholdSta
"state for %s -- got %v, want %v", g.TipName(), tipHash, tipHeight,
id, s.State, state)
}
if s.Choice != choice {
if !reflect.DeepEqual(s.Choice, choice) {
gotChoiceID, wantChoiceID := "<nil>", "<nil>"
if s.Choice != nil {
gotChoiceID = s.Choice.Id
}
if choice != nil {
wantChoiceID = choice.Id
}
g.t.Fatalf("block %q (hash %s, height %d) unexpected choice for %s -- "+
"got %v, want %v", g.TipName(), tipHash, tipHeight, id, s.Choice,
choice)
"got %v, want %v", g.TipName(), tipHash, tipHeight, id, gotChoiceID,
wantChoiceID)
}
}

Expand Down
98 changes: 47 additions & 51 deletions internal/blockchain/thresholdstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,37 +19,38 @@ type ThresholdState byte

// These constants are used to identify specific threshold states.
const (
// ThresholdDefined is the first state for each deployment and is the
// ThresholdInvalid is an invalid state and exists for use as the zero value
// in error paths.
ThresholdInvalid ThresholdState = iota

// ThresholdDefined is the initial state for each deployment and is the
// state for the genesis block has by definition for all deployments.
ThresholdDefined ThresholdState = iota
ThresholdDefined

// ThresholdStarted is the state for a deployment once its start time
// has been reached.
// ThresholdStarted is the state for a deployment once its start time has
// been reached.
ThresholdStarted

// ThresholdLockedIn is the state for a deployment during the retarget
// period which is after the ThresholdStarted state period and the
// number of blocks that have voted for the deployment equal or exceed
// the required number of votes for the deployment.
// period which is after the ThresholdStarted state period and the number of
// blocks that have voted for the deployment equal or exceed the required
// number of votes for the deployment.
ThresholdLockedIn

// ThresholdActive is the state for a deployment for all blocks after a
// retarget period in which the deployment was in the ThresholdLockedIn
// state.
ThresholdActive

// ThresholdFailed is the state for a deployment once its expiration
// time has been reached and it did not reach the ThresholdLockedIn
// state.
// ThresholdFailed is the state for a deployment once its expiration time
// has been reached and it did not reach the ThresholdLockedIn state.
ThresholdFailed

// ThresholdInvalid is a deployment that does not exist.
ThresholdInvalid
)

// thresholdStateStrings is a map of ThresholdState values back to their
// constant names for pretty printing.
var thresholdStateStrings = map[ThresholdState]string{
ThresholdInvalid: "ThresholdInvalid",
ThresholdDefined: "ThresholdDefined",
ThresholdStarted: "ThresholdStarted",
ThresholdLockedIn: "ThresholdLockedIn",
Expand All @@ -65,27 +66,31 @@ func (t ThresholdState) String() string {
return fmt.Sprintf("Unknown ThresholdState (%d)", int(t))
}

const (
// invalidChoice indicates an invalid choice in the
// ThresholdStateTuple.
invalidChoice = uint32(0xffffffff)
)

// ThresholdStateTuple contains the current state and the activated choice,
// when valid.
type ThresholdStateTuple struct {
// state contains the current ThresholdState.
// State contains the current ThresholdState.
State ThresholdState

// Choice is set to invalidChoice unless state is: ThresholdLockedIn,
// ThresholdFailed & ThresholdActive. Choice should always be crosschecked
// with invalidChoice.
Choice uint32
// Choice is the specific choice that received the majority vote for the
// ThresholdLockedIn and ThresholdActive states.
//
// IMPORTANT: It will only be set to the majority no choice for the
// ThresholdFailed state if the vote failed as the result of a majority no
// vote. Otherwise, it will be nil for the ThresholdFailed state if the
// vote failed due to the voting period expiring before any majority is
// reached. This distinction allows callers to differentiate between a vote
// failing due to a majority no vote versus due to expiring, but it does
// mean the caller must check for nil before using it for the state.
//
// It is nil for all other states.
Choice *chaincfg.Choice
}

// thresholdStateTupleStrings is a map of ThresholdState values back to their
// constant names for pretty printing.
var thresholdStateTupleStrings = map[ThresholdState]string{
ThresholdInvalid: "invalid",
ThresholdDefined: "defined",
ThresholdStarted: "started",
ThresholdLockedIn: "lockedin",
Expand All @@ -96,22 +101,19 @@ var thresholdStateTupleStrings = map[ThresholdState]string{
// String returns the ThresholdStateTuple as a human-readable tuple.
func (t ThresholdStateTuple) String() string {
if s := thresholdStateTupleStrings[t.State]; s != "" {
if t.Choice != nil {
return fmt.Sprintf("%v (choice: %v)", s, t.Choice.Id)
}
return fmt.Sprintf("%v", s)
}
return "invalid"
}

// newThresholdState returns an initialized ThresholdStateTuple.
func newThresholdState(state ThresholdState, choice uint32) ThresholdStateTuple {
func newThresholdState(state ThresholdState, choice *chaincfg.Choice) ThresholdStateTuple {
return ThresholdStateTuple{State: state, Choice: choice}
}

var (
// invalidThresholdState is a threshold state tuple that represents an
// invalid state for use with errors.
invalidThresholdState = newThresholdState(ThresholdInvalid, invalidChoice)
)

// thresholdStateCache provides a type to cache the threshold states of each
// threshold window for a set of IDs. It also keeps track of which entries have
// been modified and therefore need to be written to the database.
Expand Down Expand Up @@ -177,15 +179,13 @@ func nextDeploymentVersion(params *chaincfg.Params, version uint32) uint32 {
//
// This function MUST be called with the chain state lock held (for writes).
func (b *BlockChain) nextThresholdState(prevNode *blockNode, deployment *deploymentInfo) (ThresholdStateTuple, error) {
const funcName = "nextThresholdState"

// The threshold state for the window that contains the genesis block is
// defined by definition.
ruleChangeInterval := b.chainParams.RuleChangeActivationInterval
confirmationWindow := int64(ruleChangeInterval)
svh := b.chainParams.StakeValidationHeight
if prevNode == nil || prevNode.height+1 < svh+confirmationWindow {
return newThresholdState(ThresholdDefined, invalidChoice), nil
return newThresholdState(ThresholdDefined, nil), nil
}

// Get the ancestor that is the last block of the previous confirmation
Expand All @@ -211,13 +211,10 @@ func (b *BlockChain) nextThresholdState(prevNode *blockNode, deployment *deploym
// time, so calculate it now.
medianTime := prevNode.CalcPastMedianTime()

// The state is simply defined if the start time hasn't been
// been reached yet.
// The state is simply defined if the start time hasn't been reached
// yet.
if uint64(medianTime.Unix()) < beginTime {
cache.Update(prevNode.hash, ThresholdStateTuple{
State: ThresholdDefined,
Choice: invalidChoice,
})
cache.Update(prevNode.hash, newThresholdState(ThresholdDefined, nil))
break
}

Expand All @@ -232,20 +229,19 @@ func (b *BlockChain) nextThresholdState(prevNode *blockNode, deployment *deploym

// Start with the threshold state for the most recent confirmation
// window that has a cached state.
stateTuple := newThresholdState(ThresholdDefined, invalidChoice)
stateTuple := newThresholdState(ThresholdDefined, nil)
if prevNode != nil {
var ok bool
stateTuple, ok = cache.Lookup(prevNode.hash)
if !ok {
return newThresholdState(ThresholdFailed, invalidChoice),
AssertError(fmt.Sprintf("%s: cache lookup failed for %v",
funcName, prevNode.hash))
// A cache entry is guaranteed to exist due to the above code and
// the code below relies on it, so assert the assumption.
panicf("threshold state cache lookup failed for %v", prevNode.hash)
}
}

// Since each threshold state depends on the state of the previous
// window, iterate starting from the oldest unknown window.
version := deployment.version
endTime := deployment.deployment.ExpireTime
for neededNum := len(neededStates) - 1; neededNum >= 0; neededNum-- {
prevNode := neededStates[neededNum]
Expand All @@ -268,14 +264,14 @@ func (b *BlockChain) nextThresholdState(prevNode *blockNode, deployment *deploym
}

// Make sure we are on the correct stake version.
if b.calcStakeVersion(prevNode) < version {
if b.calcStakeVersion(prevNode) < deployment.version {
stateTuple.State = ThresholdDefined
break
}

// The state must remain in the defined state so long as
// a majority of the PoW miners have not upgraded.
if !b.isMajorityVersion(int32(version), prevNode,
if !b.isMajorityVersion(int32(deployment.version), prevNode,
b.chainParams.BlockRejectNumRequired) {

stateTuple.State = ThresholdDefined
Expand Down Expand Up @@ -365,12 +361,12 @@ func (b *BlockChain) nextThresholdState(prevNode *blockNode, deployment *deploym
switch {
case !choice.IsNo:
stateTuple.State = ThresholdLockedIn
stateTuple.Choice = uint32(choiceIdx)
stateTuple.Choice = choice
break nextChoice

case choice.IsNo:
stateTuple.State = ThresholdFailed
stateTuple.Choice = uint32(choiceIdx)
stateTuple.Choice = choice
break nextChoice
}
}
Expand Down Expand Up @@ -509,13 +505,13 @@ func (b *BlockChain) StateLastChangedHeight(hash *chainhash.Hash, deploymentID s
func (b *BlockChain) NextThresholdState(hash *chainhash.Hash, deploymentID string) (ThresholdStateTuple, error) {
node := b.index.LookupNode(hash)
if node == nil || !b.index.CanValidate(node) {
return invalidThresholdState, unknownBlockError(hash)
return ThresholdStateTuple{}, unknownBlockError(hash)
}

deployment, ok := b.deploymentData[deploymentID]
if !ok {
str := fmt.Sprintf("deployment ID %s does not exist", deploymentID)
return invalidThresholdState, contextError(ErrUnknownDeploymentID, str)
return ThresholdStateTuple{}, contextError(ErrUnknownDeploymentID, str)
}

b.chainLock.Lock()
Expand Down
Loading

0 comments on commit 59e1d24

Please sign in to comment.