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

#13 Transaction will be ignored when same source found in 'VotingBox' #49

Merged
merged 4 commits into from
Jun 8, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/ballot.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ type BallotBoxes struct {
WaitingBox *BallotBox
VotingBox *BallotBox
ReservedBox *BallotBox

Messages map[ /* `Message.GetHash()`*/ string]sebakcommon.Message
}

func NewBallotBoxes() *BallotBoxes {
Expand All @@ -275,6 +277,7 @@ func NewBallotBoxes() *BallotBoxes {
WaitingBox: NewBallotBox(),
VotingBox: NewBallotBox(),
ReservedBox: NewBallotBox(),
Messages: map[string]sebakcommon.Message{},
Copy link
Contributor

Choose a reason for hiding this comment

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

Golang question: Why isn't this using make ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@geod make will be good, it just matter of choice. :)

}
}

Expand Down Expand Up @@ -325,6 +328,7 @@ func (b *BallotBoxes) RemoveVotingResult(vr *VotingResult) (err error) {
}

delete(b.Results, vr.MessageHash)
delete(b.Messages, vr.MessageHash)

return
}
Expand Down Expand Up @@ -363,6 +367,10 @@ func (b *BallotBoxes) AddBallot(ballot Ballot) (isNew bool, err error) {
err = b.AddVotingResult(vr, b.VotingBox)
}

if _, found := b.Messages[ballot.MessageHash()]; !found {
b.Messages[ballot.MessageHash()] = ballot.Data().Data.(sebakcommon.Message)
}

return
}

Expand Down
1 change: 1 addition & 0 deletions lib/node_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func (nr *NodeRunner) ConnectValidators() {

var DefaultHandleMessageFromClientCheckerFuncs = []sebakcommon.CheckerFunc{
CheckNodeRunnerHandleMessageTransactionUnmarshal,
CheckNodeRunnerHandleMessageTransactionHasSameSource,
CheckNodeRunnerHandleMessageHistory,
CheckNodeRunnerHandleMessageISAACReceiveMessage,
CheckNodeRunnerHandleMessageSignBallot,
Expand Down
18 changes: 18 additions & 0 deletions lib/node_runner_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ func CheckNodeRunnerHandleMessageTransactionUnmarshal(c sebakcommon.Checker, arg
return
}

func CheckNodeRunnerHandleMessageTransactionHasSameSource(c sebakcommon.Checker, args ...interface{}) (err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This checker may called heavily. Hence, performance is important.

How about maintaining a list of the 'Source' instead of finding source from message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kfangw Yeah, I agree. At first time, I tried to do that as you suggested, but the changes would be huge. We'd better to handle this issues in another issue, I think. How about that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. agreed

Copy link
Member

Choose a reason for hiding this comment

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

#59

checker := c.(*NodeRunnerHandleMessageChecker)

incomingTx := checker.Transaction
isaac := checker.NodeRunner.Consensus().(*ISAAC)

for _, hash := range isaac.Boxes.VotingBox.Hashes {
existingTx := isaac.Boxes.Messages[hash].(Transaction)
if incomingTx.B.Source != existingTx.B.Source {
continue
}
err = sebakcommon.CheckerErrorStop{"stop consensus, because same source transaction already in progress"}
return
}

return
}

func CheckNodeRunnerHandleMessageHistory(c sebakcommon.Checker, args ...interface{}) (err error) {
checker := c.(*NodeRunnerHandleMessageChecker)

Expand Down
199 changes: 199 additions & 0 deletions lib/node_runner_isaac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,202 @@ func TestNodeRunnerConsensusStoreInHistoryNewBallot(t *testing.T) {
}
}
}

// TestNodeRunnerConsensusSameSourceWillBeIgnored checks, the transaction which
// has same source will be ignored if the transaction has same source and it is
// in 'SIGN' state.
func TestNodeRunnerConsensusSameSourceWillBeIgnored(t *testing.T) {
defer sebaknetwork.CleanUpMemoryNetwork()

numberOfNodes := 3
nodeRunners := createNodeRunnersWithReady(numberOfNodes)

var wg sync.WaitGroup

nr0 := nodeRunners[0]
firstTx := makeTransaction(nr0.Node().Keypair())
secondTx := makeTransaction(nr0.Node().Keypair())

var handleBallotCheckerFuncs = []sebakcommon.CheckerFunc{
CheckNodeRunnerHandleBallotIsWellformed,
CheckNodeRunnerHandleBallotCheckIsNew,
CheckNodeRunnerHandleBallotReceiveBallot,

// stop consensus when reached 'SIGN'
func(c sebakcommon.Checker, args ...interface{}) (err error) {
checker := c.(*NodeRunnerHandleBallotChecker)

if checker.Ballot.MessageHash() != firstTx.GetHash() {
return
}

if checker.VotingStateStaging.State == sebakcommon.BallotStateSIGN {
err = sebakcommon.CheckerErrorStop{"stop consensus, because it is in SIGN state"}
wg.Done()
return
}

return
},
CheckNodeRunnerHandleBallotHistory,
CheckNodeRunnerHandleBallotStore,
CheckNodeRunnerHandleBallotIsBroadcastable,
CheckNodeRunnerHandleBallotVotingHole,
CheckNodeRunnerHandleBallotBroadcast,
}

for _, nr := range nodeRunners {
nr.SetHandleBallotCheckerFuncs(nil, handleBallotCheckerFuncs...)
}

client := nr0.Network().GetClient(nr0.Node().Endpoint())

wg.Add(3)
client.SendMessage(firstTx)
wg.Wait()

isaac := nr0.Consensus().(*ISAAC)
if !isaac.HasMessage(firstTx) {
t.Error("transaction not found")
return
}

if _, ok := isaac.Boxes.Results[firstTx.GetHash()]; !ok {
t.Error("VotingResult not found")
return
}

if !isaac.Boxes.VotingBox.HasMessage(firstTx) {
t.Error("ballot not in VotingBox")
return
}

var deferFunc sebakcommon.CheckerDeferFunc = func(n int, c sebakcommon.Checker, err error) {
if err == nil {
return
}

if _, ok := err.(sebakcommon.CheckerErrorStop); ok {
wg.Done()
return
}
}

for _, nr := range nodeRunners {
nr.SetHandleMessageFromClientCheckerFuncs(deferFunc)
}

wg = sync.WaitGroup{}
wg.Add(1)
client.SendMessage(secondTx)
wg.Wait()

if isaac.HasMessage(secondTx) {
t.Error("second transaction was added as VotingResult")
return
}
}

// TestNodeRunnerConsensusSameSourceWillNotIgnored checks, the transaction which
// has same source will be ignored if the transaction has same source and it is
// not in 'SIGN' state.
func TestNodeRunnerConsensusSameSourceWillNotIgnored(t *testing.T) {
defer sebaknetwork.CleanUpMemoryNetwork()

numberOfNodes := 3
nodeRunners := createNodeRunnersWithReady(numberOfNodes)

var wg sync.WaitGroup

nr0 := nodeRunners[0]
firstTx := makeTransaction(nr0.Node().Keypair())
secondTx := makeTransaction(nr0.Node().Keypair())

var handleBallotCheckerFuncs = []sebakcommon.CheckerFunc{
CheckNodeRunnerHandleBallotIsWellformed,
CheckNodeRunnerHandleBallotCheckIsNew,
CheckNodeRunnerHandleBallotReceiveBallot,

// stop consensus when reached 'INIT'
func(c sebakcommon.Checker, args ...interface{}) (err error) {
err = sebakcommon.CheckerErrorStop{"stop consensus, because it is in INIT state"}
defer wg.Done()
return
},
CheckNodeRunnerHandleBallotHistory,
CheckNodeRunnerHandleBallotStore,
CheckNodeRunnerHandleBallotIsBroadcastable,
CheckNodeRunnerHandleBallotVotingHole,
CheckNodeRunnerHandleBallotBroadcast,
}

for _, nr := range nodeRunners {
nr.SetHandleBallotCheckerFuncs(nil, handleBallotCheckerFuncs...)
}

client := nr0.Network().GetClient(nr0.Node().Endpoint())

wg.Add(2)
client.SendMessage(firstTx)
wg.Wait()

isaac := nr0.Consensus().(*ISAAC)
if !isaac.HasMessage(firstTx) {
t.Error("transaction not found")
return
}

if _, ok := isaac.Boxes.Results[firstTx.GetHash()]; !ok {
t.Error("VotingResult not found")
return
}

if !isaac.Boxes.WaitingBox.HasMessage(firstTx) {
t.Error("ballot not in WaitingBox")
return
}

var lastVotingStateStaging []VotingStateStaging
var deferFunc sebakcommon.CheckerDeferFunc = func(n int, c sebakcommon.Checker, err error) {
if err == nil {
return
}

if _, ok := err.(sebakcommon.CheckerErrorStop); !ok {
return
}

checker := c.(*NodeRunnerHandleBallotChecker)
if checker.VotingStateStaging.IsEmpty() {
return
}

if !checker.VotingStateStaging.IsClosed() {
return
}

lastVotingStateStaging = append(lastVotingStateStaging, checker.VotingStateStaging)
wg.Done()
}

for _, nr := range nodeRunners {
nr.SetHandleBallotCheckerFuncs(deferFunc, DefaultHandleBallotCheckerFuncs...)
}

wg = sync.WaitGroup{}
wg.Add(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does firstTx need only wg.Add(2) while secondTx has wg.Add(3)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@leejames00 This is the interesting part. The reason wg.Add count for firstTx is 2 is that, the handleBallotCheckerFuncs is for the other nodes except nr0. The ballot of nr0 is already INIT.

client.SendMessage(secondTx)
wg.Wait()

if len(lastVotingStateStaging) != 3 {
t.Error("failed to get consensus")
return
}

for _, vs := range lastVotingStateStaging {
if !vs.IsClosed() {
t.Error("failed to close consensus")
return
}
}
}