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

Resolve long time consensus stuck by network and simple refactorings #915

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 2 additions & 21 deletions lib/ballot/ballot.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package ballot

import (
"encoding/json"
"time"
Copy link
Contributor

Choose a reason for hiding this comment

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

I love this commit


"github.com/btcsuite/btcutil/base58"

Expand Down Expand Up @@ -106,15 +105,7 @@ func (b Ballot) isBallotWellFormed(conf common.Config) (err error) {
return
}

var confirmed time.Time
if confirmed, err = common.ParseISO8601(b.B.Confirmed); err != nil {
return
}
now := time.Now()
timeStart := now.Add(time.Duration(-1) * common.BallotConfirmedTimeAllowDuration)
timeEnd := now.Add(common.BallotConfirmedTimeAllowDuration)
if confirmed.Before(timeStart) || confirmed.After(timeEnd) {
err = errors.MessageHasIncorrectTime
if err = CheckHasCorrectTime(b.B.Confirmed); err != nil {
return
}

Expand All @@ -126,17 +117,7 @@ func (b Ballot) isBallotWellFormed(conf common.Config) (err error) {
}

func (b Ballot) isProposerInfoWellFormed(conf common.Config) (err error) {
var proposerConfirmed time.Time
if proposerConfirmed, err = common.ParseISO8601(b.ProposerConfirmed()); err != nil {
return
}

now := time.Now()
timeStart := now.Add(time.Duration(-1) * common.BallotConfirmedTimeAllowDuration)
timeEnd := now.Add(common.BallotConfirmedTimeAllowDuration)

if proposerConfirmed.Before(timeStart) || proposerConfirmed.After(timeEnd) {
err = errors.MessageHasIncorrectTime
if err = CheckHasCorrectTime(b.ProposerConfirmed()); err != nil {
return
}

Expand Down
8 changes: 3 additions & 5 deletions lib/ballot/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,15 @@ import (
)

func CheckHasCorrectTime(timeStr string) error {
var proposerConfirmed time.Time
var t time.Time
var err error
if proposerConfirmed, err = common.ParseISO8601(timeStr); err != nil {
if t, err = common.ParseISO8601(timeStr); err != nil {
return err
}

now := time.Now()
timeStart := now.Add(time.Duration(-1) * common.BallotConfirmedTimeAllowDuration)
timeEnd := now.Add(common.BallotConfirmedTimeAllowDuration)

if proposerConfirmed.Before(timeStart) || proposerConfirmed.After(timeEnd) {
if t.Before(timeStart) || t.After(timeEnd) {
return errors.MessageHasIncorrectTime
}

Expand Down
13 changes: 13 additions & 0 deletions lib/consensus/ballot_send_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ func NewBallotSendRecord(nodeAlias string) *BallotSendRecord {
return p
}

// SetSent sets that the ballot of this ISAACState has already been sent.
// This is to prevent one node from retransmitting another result.
Copy link
Contributor

Choose a reason for hiding this comment

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

Big big 👍 for documenting what the function is for. It's much more important than what the function do (which one can understand from the code).

func (r *BallotSendRecord) SetSent(state ISAACState) {
r.Lock()
defer r.Unlock()
Expand All @@ -33,6 +35,17 @@ func (r *BallotSendRecord) SetSent(state ISAACState) {
return
}

// InitSent initializes the ballot transfer record of this ISAACState.InitSent.
// This function is used when an existing ballot has expired.
func (r *BallotSendRecord) InitSent(state ISAACState) {
r.Lock()
defer r.Unlock()
log.Debug("BallotSendRecord.InitSent()", "state", state)
r.record[state] = false

return
}

func (r *BallotSendRecord) Sent(state ISAACState) bool {
r.RLock()
defer r.RUnlock()
Expand Down
87 changes: 81 additions & 6 deletions lib/consensus/isaac.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,41 @@ func (is *ISAAC) SelectProposer(blockHeight uint64, round uint64) string {
return is.proposerSelector.Select(blockHeight, round)
}

// GenerateExpiredBallot create an expired ballot using voting.Basis and ballot.State.
// This function is used to create a ballot indicating
// that a node has expired to other nodes when a timeout occurs in the state.
func (is *ISAAC) GenerateExpiredBallot(basis voting.Basis, state ballot.State) (ballot.Ballot, error) {
is.log.Debug("ISAAC.GenerateExpiredBallot", "basis", basis, "state", state)
proposerAddr := is.SelectProposer(basis.Height, basis.Round)

newExpiredBallot := ballot.NewBallot(is.Node.Address(), proposerAddr, basis, []string{})
newExpiredBallot.SetVote(state, voting.EXP)

config := is.Conf
var err error

opc, err := ballot.NewCollectTxFeeFromBallot(*newExpiredBallot, config.CommonAccountAddress)
if err != nil {
return ballot.Ballot{}, err
}

opi, err := ballot.NewInflationFromBallot(*newExpiredBallot, config.CommonAccountAddress, config.InitialBalance)
if err != nil {
return ballot.Ballot{}, err
}

ptx, err := ballot.NewProposerTransactionFromBallot(*newExpiredBallot, opc, opi)
if err != nil {
return ballot.Ballot{}, err
}

newExpiredBallot.SetProposerTransaction(ptx)
newExpiredBallot.SignByProposer(is.Node.Keypair(), config.NetworkID)
newExpiredBallot.Sign(is.Node.Keypair(), config.NetworkID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we always need to call Sign after SignByProposer, SignByProposer should probably do it itself.
That will also avoid the double hashing / signature which is expensive.

Copy link
Member Author

Choose a reason for hiding this comment

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

hm.. already SignByProposer is called in Sign.
I need more time to think about handing and signing ExpiredBallot.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Geod24 How about handling it in the next pr?
I think it is not related with this commit :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

Copy link
Member Author

Choose a reason for hiding this comment

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


return *newExpiredBallot, nil
}

//
// Check if `basis` is a valid one for the current round of consensus
//
Expand Down Expand Up @@ -206,14 +241,55 @@ func (is *ISAAC) Vote(b ballot.Ballot) (isNew bool, err error) {
return
}

func (is *ISAAC) CanGetVotingResult(b ballot.Ballot) (rv RoundVoteResult, vh voting.Hole, finished bool) {
// RemoveOldBallots checks that `blt` has valid confirmed and proposed time.
// If it is invalid, it is removed.
// And if the invalid ballot is make by itself,
// return `needRenewal` = true for rebroadcasting EXP ballot
func (is *ISAAC) RemoveOldBallots(blt ballot.Ballot) (needRenewal bool) {
runningRound, found := is.RunningRounds[blt.VotingBasis().Index()]
if !found {
// if RunningRound is not found, this ballot will be stopped.
return false
}
roundVote, err := runningRound.RoundVote(blt.Proposer())
if err != nil {
return false
}

var vr *RoundVoteResult

switch blt.State() {
case ballot.StateSIGN:
vr = &roundVote.SIGN
case ballot.StateACCEPT:
vr = &roundVote.ACCEPT
default:
return false
}

for nodeAddr, blt := range *vr {
if err = ballot.CheckHasCorrectTime(blt.ProposerConfirmed()); err == nil {
if err = ballot.CheckHasCorrectTime(blt.Confirmed()); err == nil {
continue
}
}
delete(*vr, nodeAddr)
if nodeAddr == is.Node.Address() {
needRenewal = true
}
}

return needRenewal
}

func (is *ISAAC) CanGetVotingResult(blt ballot.Ballot) (rv RoundVoteResult, vh voting.Hole, finished bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for using at least 3 char identifiers. Makes the code much more readable IMO.

is.RLock()
defer is.RUnlock()

defer func() {
is.log.Debug(
"CanGetVotingResult",
"ballot", b.GetHash(),
"ballot", blt.GetHash(),
"round-vote-result", rv,
"voting-hole", vh,
"finished", finished,
Expand All @@ -224,17 +300,16 @@ func (is *ISAAC) CanGetVotingResult(b ballot.Ballot) (rv RoundVoteResult, vh vot
vh = voting.NOTYET
finished = false

runningRound, found := is.RunningRounds[b.VotingBasis().Index()]
runningRound, found := is.RunningRounds[blt.VotingBasis().Index()]
if !found {
// if RunningRound is not found, this ballot will be stopped.
finished = true
return
}

roundVote, err := runningRound.RoundVote(b.Proposer())
roundVote, err := runningRound.RoundVote(blt.Proposer())
if err == nil {
rv, vh, finished = roundVote.CanGetVotingResult(is.policy, b.State(), is.log)
return
rv, vh, finished = roundVote.CanGetVotingResult(is.policy, blt.State(), is.log)
}

return
Expand Down
18 changes: 10 additions & 8 deletions lib/consensus/round_vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"boscoin.io/sebak/lib/voting"
)

type RoundVoteResult map[ /* Node.Address() */ string]voting.Hole
type RoundVoteResult map[ /* Node.Address() */ string]ballot.Ballot

type RoundVote struct {
SIGN RoundVoteResult
Expand Down Expand Up @@ -40,13 +40,15 @@ func (rv *RoundVote) IsVotedByNode(state ballot.State, node string) bool {
}

func (rv *RoundVote) Vote(b ballot.Ballot) (isNew bool, err error) {
if b.State() == ballot.StateSIGN || b.State() == ballot.StateACCEPT {
result := rv.GetResult(b.State())

_, isNew = result[b.Source()]
result[b.Source()] = b.Vote()
if !b.State().IsValidForVote() {
return
}

result := rv.GetResult(b.State())

_, isNew = result[b.Source()]
result[b.Source()] = b

return
}

Expand Down Expand Up @@ -74,8 +76,8 @@ func (rv *RoundVote) CanGetVotingResult(policy voting.ThresholdPolicy, state bal
result := rv.GetResult(state)

var yes, no, expired int
for _, votingHole := range result {
switch votingHole {
for _, blt := range result {
switch blt.Vote() {
case voting.YES:
yes++
case voting.NO:
Expand Down
41 changes: 41 additions & 0 deletions lib/node/runner/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ func BallotUnmarshal(c common.Checker, args ...interface{}) (err error) {
}

if err = b.IsWellFormed(checker.Conf); err != nil {
if err == errors.MessageHasIncorrectTime && b.State().IsValidForVote() {
blt, ierr := checker.NodeRunner.Consensus().GenerateExpiredBallot(b.VotingBasis(), b.State())
if ierr != nil {
checker.NodeRunner.Log().Error("an error generating an expired ballot", "err", ierr)
} else {
checker.NodeRunner.BroadcastBallot(blt)
}
}
return
}

Expand Down Expand Up @@ -376,6 +384,39 @@ func BallotIsSameProposer(c common.Checker, args ...interface{}) (err error) {
return
}

// BallotRenewal checks that the ballot confirmed and proposed time is valid and
// if the time is invalid, then it broadcasts expired ballot
func BallotRenewal(c common.Checker, args ...interface{}) (err error) {
checker := c.(*BallotChecker)
nr := checker.NodeRunner
is := nr.Consensus()
blt := checker.Ballot

if !blt.State().IsValidForVote() {
return
}

needRenewal := is.RemoveOldBallots(blt)
if needRenewal {
expiredBlt, err := is.GenerateExpiredBallot(blt.VotingBasis(), blt.State())
if err != nil {
return err
}
nr.InitSent(
// SentRecord should be initialized,
// because the renewal ballot has to replace the old ballot.
consensus.ISAACState{
Height: blt.VotingBasis().Height,
Round: blt.VotingBasis().Round,
BallotState: blt.State(),
},
)
nr.BroadcastBallot(expiredBlt)
}

return
}

// BallotCheckResult checks the voting result.
func BallotCheckResult(c common.Checker, args ...interface{}) (err error) {
checker := c.(*BallotChecker)
Expand Down
21 changes: 7 additions & 14 deletions lib/node/runner/isaac_state_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,21 +263,14 @@ func (sm *ISAACStateManager) broadcastExpiredBallot(round uint64, state ballot.S
TotalOps: b.TotalOps,
}

proposerAddr := sm.nr.consensus.SelectProposer(b.Height, round)

newExpiredBallot := ballot.NewBallot(sm.nr.localNode.Address(), proposerAddr, basis, []string{})
newExpiredBallot.SetVote(state, voting.EXP)

opc, _ := ballot.NewCollectTxFeeFromBallot(*newExpiredBallot, sm.nr.Conf.CommonAccountAddress)
opi, _ := ballot.NewInflationFromBallot(*newExpiredBallot, sm.nr.Conf.CommonAccountAddress, sm.nr.Conf.InitialBalance)
ptx, _ := ballot.NewProposerTransactionFromBallot(*newExpiredBallot, opc, opi)

newExpiredBallot.SetProposerTransaction(ptx)
newExpiredBallot.SignByProposer(sm.nr.localNode.Keypair(), sm.nr.Conf.NetworkID)
newExpiredBallot.Sign(sm.nr.localNode.Keypair(), sm.nr.Conf.NetworkID)
newExpiredBallot, err := sm.nr.consensus.GenerateExpiredBallot(basis, state)
if err != nil {
sm.nr.Log().Error("an error generating an expired ballot", "err", err)
} else {
sm.nr.BroadcastBallot(newExpiredBallot)
}

sm.nr.Log().Debug("broadcast", "ballot", *newExpiredBallot)
sm.nr.BroadcastBallot(*newExpiredBallot)
return
}

func (sm *ISAACStateManager) resetTimer(timer *time.Timer, state ballot.State) {
Expand Down
6 changes: 6 additions & 0 deletions lib/node/runner/node_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ var DefaultHandleSIGNBallotCheckerFuncs = []common.CheckerFunc{
BallotAlreadyVoted,
BallotVote,
BallotIsSameProposer,
BallotRenewal,
BallotCheckResult,
ExpiredInSIGN,
ACCEPTBallotBroadcast,
Expand All @@ -64,6 +65,7 @@ var DefaultHandleACCEPTBallotCheckerFuncs = []common.CheckerFunc{
BallotAlreadyVoted,
BallotVote,
BallotIsSameProposer,
BallotRenewal,
BallotCheckResult,
FinishedBallotStore,
}
Expand Down Expand Up @@ -770,3 +772,7 @@ func (nr *NodeRunner) BroadcastBallot(b ballot.Ballot) {

nr.ConnectionManager().Broadcast(b)
}

func (nr *NodeRunner) InitSent(state consensus.ISAACState) {
nr.ballotSendRecord.InitSent(state)
}
Loading