Skip to content

Commit

Permalink
fix!: updating sortition executor (#608)
Browse files Browse the repository at this point in the history
* fix!: updating sortition executor

* chore: fixing linting issues

* fix: fixing review comments
  • Loading branch information
themantre authored Jul 30, 2023
1 parent 239bb98 commit d30f0a2
Show file tree
Hide file tree
Showing 33 changed files with 651 additions and 523 deletions.
8 changes: 4 additions & 4 deletions committee/committee.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (c *committee) Update(lastRound int16, joined []*validator.Validator) {
if committeeVal == nil {
c.validatorList.InsertBefore(cloneValidator(val), c.proposerPos)
} else {
committeeVal.UpdateLastJoinedHeight(val.LastJoinedHeight())
committeeVal.UpdateLastSortitionHeight(val.LastSortitionHeight())

// Ensure that a validator's stake and bonding properties
// remain unchanged while they are part of the committee.
Expand All @@ -89,8 +89,8 @@ func (c *committee) Update(lastRound int16, joined []*validator.Validator) {
}

sort.SliceStable(oldestFirst, func(i, j int) bool {
return oldestFirst[i].Value.(*validator.Validator).LastJoinedHeight() <
oldestFirst[j].Value.(*validator.Validator).LastJoinedHeight()
return oldestFirst[i].Value.(*validator.Validator).LastSortitionHeight() <
oldestFirst[j].Value.(*validator.Validator).LastSortitionHeight()
})

for i := 0; i <= int(lastRound); i++ {
Expand Down Expand Up @@ -187,7 +187,7 @@ func (c *committee) String() string {

builder.WriteString("[ ")
for _, v := range c.Validators() {
builder.WriteString(fmt.Sprintf("%v(%v)", v.Number(), v.LastJoinedHeight()))
builder.WriteString(fmt.Sprintf("%v(%v)", v.Number(), v.LastSortitionHeight()))
if c.IsProposer(v.Address(), 0) {
builder.WriteString("*")
}
Expand Down
34 changes: 17 additions & 17 deletions committee/committee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,15 @@ func TestProposerJoin(t *testing.T) {

// Height 1000
// Val2 is in the committee
val2.UpdateLastJoinedHeight(1000)
val2.UpdateLastSortitionHeight(1000)
committee.Update(0, []*validator.Validator{val2})
assert.Equal(t, committee.Proposer(0).Number(), int32(2))
assert.Equal(t, committee.Committers(), []int32{1, 2, 3, 4})
assert.Equal(t, committee.Size(), 4)
fmt.Println(committee.String())

// Height 1001
val5.UpdateLastJoinedHeight(1001)
val5.UpdateLastSortitionHeight(1001)
committee.Update(0, []*validator.Validator{val5})
assert.Equal(t, committee.Proposer(0).Number(), int32(3))
assert.Equal(t, committee.Proposer(1).Number(), int32(4))
Expand All @@ -163,9 +163,9 @@ func TestProposerJoin(t *testing.T) {
fmt.Println(committee.String())

// Height 1003
val3.UpdateLastJoinedHeight(1003)
val6.UpdateLastJoinedHeight(1003)
val7.UpdateLastJoinedHeight(1003)
val3.UpdateLastSortitionHeight(1003)
val6.UpdateLastSortitionHeight(1003)
val7.UpdateLastSortitionHeight(1003)
committee.Update(1, []*validator.Validator{val7, val3, val6})
assert.Equal(t, committee.Proposer(0).Number(), int32(2))
assert.Equal(t, committee.Committers(), []int32{6, 7, 1, 5, 2, 3, 4})
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestProposerJoinAndLeave(t *testing.T) {
// +-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+ +$+-+-+-+$+-+$+

// Height 1001
val8.UpdateLastJoinedHeight(1001)
val8.UpdateLastSortitionHeight(1001)
committee.Update(0, []*validator.Validator{val8})
assert.Equal(t, committee.Proposer(0).Number(), int32(2))
assert.Equal(t, committee.Proposer(1).Number(), int32(3))
Expand All @@ -267,30 +267,30 @@ func TestProposerJoinAndLeave(t *testing.T) {
fmt.Println(committee.String())

// Height 1002
val3.UpdateLastJoinedHeight(1002)
val3.UpdateLastSortitionHeight(1002)
committee.Update(3, []*validator.Validator{val3})
assert.Equal(t, committee.Proposer(0).Number(), int32(6))
fmt.Println(committee.String())

// Height 1003
val2.UpdateLastJoinedHeight(1003)
val9.UpdateLastJoinedHeight(1003)
val2.UpdateLastSortitionHeight(1003)
val9.UpdateLastSortitionHeight(1003)
committee.Update(0, []*validator.Validator{val9, val2})
assert.Equal(t, committee.Proposer(0).Number(), int32(7))
assert.Equal(t, committee.Proposer(1).Number(), int32(8))
assert.Equal(t, committee.Committers(), []int32{8, 2, 3, 5, 9, 6, 7})
fmt.Println(committee.String())

// Height 1004
valA.UpdateLastJoinedHeight(1004)
valA.UpdateLastSortitionHeight(1004)
committee.Update(1, []*validator.Validator{valA})
assert.Equal(t, committee.Proposer(0).Number(), int32(2))
assert.Equal(t, committee.Committers(), []int32{8, 2, 3, 9, 6, 10, 7})
fmt.Println(committee.String())

// Height 1005
valB.UpdateLastJoinedHeight(1005)
valC.UpdateLastJoinedHeight(1005)
valB.UpdateLastSortitionHeight(1005)
valC.UpdateLastSortitionHeight(1005)
committee.Update(0, []*validator.Validator{valC, valB})
assert.Equal(t, committee.Proposer(0).Number(), int32(3))
assert.Equal(t, committee.Proposer(1).Number(), int32(9))
Expand All @@ -299,17 +299,17 @@ func TestProposerJoinAndLeave(t *testing.T) {
fmt.Println(committee.String())

// Height 1006
val1.UpdateLastJoinedHeight(1006)
val1.UpdateLastSortitionHeight(1006)
committee.Update(2, []*validator.Validator{val1})
assert.Equal(t, committee.Proposer(0).Number(), int32(11))
assert.Equal(t, committee.Committers(), []int32{11, 12, 2, 1, 3, 9, 10})
fmt.Println(committee.String())

// Height 1007
val2.UpdateLastJoinedHeight(1007)
val3.UpdateLastJoinedHeight(1007)
val5.UpdateLastJoinedHeight(1007)
val6.UpdateLastJoinedHeight(1007)
val2.UpdateLastSortitionHeight(1007)
val3.UpdateLastSortitionHeight(1007)
val5.UpdateLastSortitionHeight(1007)
val6.UpdateLastSortitionHeight(1007)
committee.Update(4, []*validator.Validator{val2, val3, val5, val6})
assert.Equal(t, committee.Proposer(0).Number(), int32(5))
assert.Equal(t, committee.Committers(), []int32{5, 6, 11, 12, 2, 1, 3})
Expand Down
36 changes: 24 additions & 12 deletions execution/execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,76 +34,88 @@ func TestExecution(t *testing.T) {

t.Run("Invalid transaction, Should returns error", func(t *testing.T) {
trx, _ := ts.GenerateTestTransferTx()
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidTx)
assert.Zero(t, exe.AccumulatedFee())
})

t.Run("Genesis stamp (expired), Should returns error", func(t *testing.T) {
trx := tx.NewTransferTx(hash.UndefHash.Stamp(), 1, addr1, rcvAddr, 1000, 1000, "expired-stamp")
signer1.SignMsg(trx)
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidTx)
})

t.Run("Expired stamp, Should returns error", func(t *testing.T) {
trx := tx.NewTransferTx(block1.Stamp(), 1, addr1, rcvAddr, 1000, 1000,
"expired-stamp")
signer1.SignMsg(trx)
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidTx)
})

t.Run("stamp is valid", func(t *testing.T) {
trx := tx.NewTransferTx(block3.Stamp(), 1, addr1, rcvAddr, 1000, 1000, "ok")
signer1.SignMsg(trx)
assert.NoError(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.NoError(t, err)
})

t.Run("Subsidy transaction has an invalid stamp", func(t *testing.T) {
trx := tx.NewSubsidyTx(block8641.Stamp(), 1, rcvAddr, 1000,
"expired-stamp")
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidTx)
})

t.Run("Subsidy stamp is ok", func(t *testing.T) {
trx := tx.NewSubsidyTx(block8642.Stamp(), 1, rcvAddr, 1000, "ok")
assert.NoError(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.NoError(t, err)
})

t.Run("Invalid fee, Should returns error", func(t *testing.T) {
trx := tx.NewTransferTx(block3.Stamp(), 2, addr1, rcvAddr, 1000, 1, "invalid fee")
signer1.SignMsg(trx)
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidFee)
})

t.Run("Invalid fee, Should returns error", func(t *testing.T) {
trx := tx.NewTransferTx(block3.Stamp(), 2, addr1, rcvAddr, 1000, 1001, "invalid fee")
signer1.SignMsg(trx)
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidFee)
})

t.Run("Invalid fee (subsidy tx), Should returns error", func(t *testing.T) {
trx := tx.NewTransferTx(block3.Stamp(), 2, crypto.TreasuryAddress, rcvAddr, 1000, 1, "invalid fee")
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidFee)
assert.Error(t, exe.checkFee(trx, sb))
})

t.Run("Invalid fee (send tx), Should returns error", func(t *testing.T) {
trx := tx.NewTransferTx(block3.Stamp(), 2, addr1, rcvAddr, 1000, 0, "invalid fee")
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidFee)
assert.Error(t, exe.checkFee(trx, sb))
})

t.Run("Sortition tx - Expired stamp, Should returns error", func(t *testing.T) {
proof := ts.RandomProof()
trx := tx.NewSortitionTx(block8635.Stamp(), 1, addr1, proof)
signer1.SignMsg(trx)
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidTx)
})

t.Run("Execution failed", func(t *testing.T) {
proof := ts.RandomProof()
trx := tx.NewSortitionTx(block8642.Stamp(), 1, addr1, proof)
signer1.SignMsg(trx)
assert.Error(t, exe.Execute(trx, sb))
err := exe.Execute(trx, sb)
assert.Equal(t, errors.Code(err), errors.ErrInvalidAddress)
})
}

Expand Down
12 changes: 6 additions & 6 deletions execution/executor/bond.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,19 @@ func (e *BondExecutor) Execute(trx *tx.Tx, sb sandbox.Sandbox) error {
}
if e.strict {
// In strict mode, bond transactions will be rejected if a validator is
// in the committee.
// In non-strict mode, we accept them and keep them inside the transaction pool
// already in the committee.
// In non-strict mode, we accept them and keep them in the transaction pool
// to process them when the validator leaves the committee.
if sb.Committee().Contains(pld.Receiver) {
return errors.Errorf(errors.ErrInvalidTx,
"validator %v is in committee", pld.Receiver)
}

// In strict mode, bond transactions will be rejected if a validator is
// going to be in the committee for the next height.
// In non-strict mode, we accept it and keep it inside the transaction pool to
// going to join the committee in the next height.
// In non-strict mode, we accept it and keep it in the transaction pool to
// process it when the validator leaves the committee.
if receiverVal.LastJoinedHeight() == sb.CurrentHeight() {
if sb.IsJoinedCommittee(pld.Receiver) {
return errors.Errorf(errors.ErrInvalidTx,
"validator %v joins committee in the next height", pld.Receiver)
}
Expand All @@ -68,7 +68,7 @@ func (e *BondExecutor) Execute(trx *tx.Tx, sb sandbox.Sandbox) error {
return errors.Error(errors.ErrInsufficientFunds)
}
if receiverVal.Stake()+pld.Stake > sb.Params().MaximumStake {
return errors.Errorf(errors.ErrInvalidTx,
return errors.Errorf(errors.ErrInvalidAmount,
"validator's stake can't be more than %v", sb.Params().MaximumStake)
}

Expand Down
Loading

0 comments on commit d30f0a2

Please sign in to comment.