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

move IsValidCandidateName to action package #3706

Merged
merged 5 commits into from
Dec 8, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
16 changes: 16 additions & 0 deletions action/candidate_register.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ var (

// ErrInvalidAmount represents that amount is 0 or negative
ErrInvalidAmount = errors.New("invalid amount")

//ErrInvalidCanName represents that candidate name is invalid
ErrInvalidCanName = errors.New("invalid candidate name")
)

// CandidateRegister is the action to register a candidate
Expand Down Expand Up @@ -380,3 +383,16 @@ func (cr *CandidateRegister) ToEthTx() (*types.Transaction, error) {
}
return types.NewTransaction(cr.Nonce(), ethAddr, big.NewInt(0), cr.GasLimit(), cr.GasPrice(), data), nil
}

// IsValidCandidateName check if a candidate name string is valid.
func IsValidCandidateName(s string) bool {
if len(s) == 0 || len(s) > 12 {
return false
}
for _, c := range s {
if !(('a' <= c && c <= 'z') || ('0' <= c && c <= '9')) {
return false
}
}
return true
}
46 changes: 46 additions & 0 deletions action/candidateregister_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,49 @@ func TestCandidateRegisterABIEncodeAndDecode(t *testing.T) {
require.Equal(test.AutoStake, stake.AutoStake())
require.Equal(test.Payload, stake.Payload())
}

func TestIsValidCandidateName(t *testing.T) {
require := require.New(t)
tests := []struct {
input string
output bool
}{
{
input: "abc",
output: true,
},
{
input: "123",
output: true,
},
{
input: "abc123abc123",
output: true,
},
{
input: "Abc123",
output: false,
},
{
input: "Abc 123",
output: false,
},
{
input: "Abc-123",
output: false,
},
{
input: "abc123abc123abc123",
output: false,
},
{
input: "",
output: false,
},
}

for _, tt := range tests {
output := IsValidCandidateName(tt.input)
require.Equal(tt.output, output)
}
}
5 changes: 3 additions & 2 deletions action/protocol/staking/candidate.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/pkg/errors"
"google.golang.org/protobuf/proto"

"github.com/iotexproject/iotex-core/action"
"github.com/iotexproject/iotex-core/action/protocol/staking/stakingpb"
"github.com/iotexproject/iotex-core/state"
)
Expand Down Expand Up @@ -73,7 +74,7 @@ func (d *Candidate) Validate() error {
}

if d.Name == "" {
return ErrInvalidCanName
return action.ErrInvalidCanName
}

if d.Owner == nil {
Expand All @@ -100,7 +101,7 @@ func (d *Candidate) Collision(c *Candidate) error {
return nil
}
if c.Name == d.Name {
return ErrInvalidCanName
return action.ErrInvalidCanName
}
if address.Equal(c.Operator, d.Operator) {
return ErrInvalidOperator
Expand Down
3 changes: 2 additions & 1 deletion action/protocol/staking/candidate_center.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/iotexproject/iotex-address/address"

"github.com/iotexproject/iotex-core/action"
"github.com/iotexproject/iotex-core/action/protocol"
)

Expand Down Expand Up @@ -294,7 +295,7 @@ func (m *CandidateCenter) collision(d *Candidate) error {

name, oper, self := m.base.collision(d)
if name != nil && !m.change.containsOwner(name) {
return ErrInvalidCanName
return action.ErrInvalidCanName
}

if oper != nil && !m.change.containsOwner(oper) {
Expand Down
11 changes: 6 additions & 5 deletions action/protocol/staking/candidate_center_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/stretchr/testify/require"

"github.com/iotexproject/iotex-core/action"
"github.com/iotexproject/iotex-core/action/protocol"
"github.com/iotexproject/iotex-core/pkg/unit"
"github.com/iotexproject/iotex-core/test/identityset"
Expand Down Expand Up @@ -241,7 +242,7 @@ func TestCandCenter(t *testing.T) {
// d1 conflict with existing
conflict := list[(u1+3)%size]
d1.Name = conflict.Name
r.Equal(ErrInvalidCanName, m.Upsert(d1))
r.Equal(action.ErrInvalidCanName, m.Upsert(d1))
d1.Name = name1
d1.Operator = conflict.Operator
r.Equal(ErrInvalidOperator, m.Upsert(d1))
Expand All @@ -261,7 +262,7 @@ func TestCandCenter(t *testing.T) {

// d2 conflict d1
d2.Name = d1.Name
r.Equal(ErrInvalidCanName, m.Upsert(d2))
r.Equal(action.ErrInvalidCanName, m.Upsert(d2))
d2.Name = name2
d2.Operator = d1.Operator
r.Equal(ErrInvalidOperator, m.Upsert(d2))
Expand All @@ -278,7 +279,7 @@ func TestCandCenter(t *testing.T) {
// new n1 conflict with existing
n1 := list[(u2+size/2)%size].Clone()
n1.Owner = identityset.Address(15 + i*2)
r.Equal(ErrInvalidCanName, m.Upsert(n1))
r.Equal(action.ErrInvalidCanName, m.Upsert(n1))
n1.Name = name1
r.Equal(ErrInvalidOperator, m.Upsert(n1))
n1.Operator = op1
Expand All @@ -290,7 +291,7 @@ func TestCandCenter(t *testing.T) {
// new n2 conflict with dirty d2
n2 := d2.Clone()
n2.Owner = identityset.Address(16 + i*2)
r.Equal(ErrInvalidCanName, m.Upsert(n2))
r.Equal(action.ErrInvalidCanName, m.Upsert(n2))
n2.Name = name2
r.Equal(ErrInvalidOperator, m.Upsert(n2))
n2.Operator = op2
Expand All @@ -302,7 +303,7 @@ func TestCandCenter(t *testing.T) {
// verify conflict with n1
n2 = n1.Clone()
n2.Owner = identityset.Address(0)
r.Equal(ErrInvalidCanName, m.Upsert(n2))
r.Equal(action.ErrInvalidCanName, m.Upsert(n2))
n2.Name = "noconflict"
r.Equal(ErrInvalidOperator, m.Upsert(n2))
n2.Operator = identityset.Address(0)
Expand Down
3 changes: 2 additions & 1 deletion action/protocol/staking/candidate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/pkg/errors"
"github.com/stretchr/testify/require"

"github.com/iotexproject/iotex-core/action"
"github.com/iotexproject/iotex-core/pkg/unit"
"github.com/iotexproject/iotex-core/state"
"github.com/iotexproject/iotex-core/test/identityset"
Expand Down Expand Up @@ -89,7 +90,7 @@ func TestClone(t *testing.T) {
r.False(d.Equal(d2))
r.NoError(d.Collision(d2))
d.Owner = identityset.Address(0)
r.Equal(ErrInvalidCanName, d.Collision(d2))
r.Equal(action.ErrInvalidCanName, d.Collision(d2))
d.Name = "noconflict"
r.Equal(ErrInvalidOperator, d.Collision(d2))
d.Operator = identityset.Address(0)
Expand Down
4 changes: 2 additions & 2 deletions action/protocol/staking/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ func (p *Protocol) handleCandidateRegister(ctx context.Context, act *action.Cand
// cannot collide with existing name
if csm.ContainsName(act.Name()) && (!ownerExist || act.Name() != c.Name) {
return log, nil, &handleError{
err: ErrInvalidCanName,
err: action.ErrInvalidCanName,
failureStatus: iotextypes.ReceiptStatus_ErrCandidateConflict,
}
}
Expand Down Expand Up @@ -843,7 +843,7 @@ func csmErrorToHandleError(caller string, err error) error {
}

switch errors.Cause(err) {
case ErrInvalidCanName:
case action.ErrInvalidCanName:
hErr.failureStatus = iotextypes.ReceiptStatus_ErrCandidateConflict
return hErr
case ErrInvalidOperator:
Expand Down
12 changes: 6 additions & 6 deletions action/protocol/staking/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func TestProtocol_HandleCreateStake(t *testing.T) {
1,
time.Now(),
10000,
ErrInvalidCanName,
action.ErrInvalidCanName,
iotextypes.ReceiptStatus_ErrCandidateNotExist,
},
{
Expand Down Expand Up @@ -379,7 +379,7 @@ func TestProtocol_HandleCandidateRegister(t *testing.T) {
uint64(1000000),
big.NewInt(1),
true,
ErrInvalidCanName,
action.ErrInvalidCanName,
iotextypes.ReceiptStatus_Failure,
},
// success for the following test
Expand Down Expand Up @@ -762,7 +762,7 @@ func TestProtocol_HandleCandidateUpdate(t *testing.T) {
"!invalidname",
identityset.Address(31).String(),
identityset.Address(32).String(),
ErrInvalidCanName,
action.ErrInvalidCanName,
iotextypes.ReceiptStatus_Failure,
},
// success,update name, operator and reward address
Expand Down Expand Up @@ -1522,7 +1522,7 @@ func TestProtocol_HandleChangeCandidate(t *testing.T) {
time.Now(),
10000,
false,
ErrInvalidCanName,
action.ErrInvalidCanName,
iotextypes.ReceiptStatus_Failure,
},
// invalid candidate name 2
Expand All @@ -1542,7 +1542,7 @@ func TestProtocol_HandleChangeCandidate(t *testing.T) {
time.Now(),
10000,
false,
ErrInvalidCanName,
action.ErrInvalidCanName,
iotextypes.ReceiptStatus_Failure,
},
// invalid candidate name 3
Expand All @@ -1562,7 +1562,7 @@ func TestProtocol_HandleChangeCandidate(t *testing.T) {
time.Now(),
10000,
false,
ErrInvalidCanName,
action.ErrInvalidCanName,
iotextypes.ReceiptStatus_Failure,
},
// Upsert error cannot happen,because CreateStake already check collision
Expand Down
30 changes: 8 additions & 22 deletions action/protocol/staking/validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
// Errors
var (
ErrInvalidAmount = errors.New("invalid staking amount")
millken marked this conversation as resolved.
Show resolved Hide resolved
ErrInvalidCanName = errors.New("invalid candidate name")
ErrInvalidOwner = errors.New("invalid owner address")
ErrInvalidOperator = errors.New("invalid operator address")
ErrInvalidReward = errors.New("invalid reward address")
Expand All @@ -27,8 +26,8 @@ var (
)

func (p *Protocol) validateCreateStake(ctx context.Context, act *action.CreateStake) error {
if !isValidCandidateName(act.Candidate()) {
return ErrInvalidCanName
if !action.IsValidCandidateName(act.Candidate()) {
return action.ErrInvalidCanName
}
if act.Amount().Cmp(p.config.MinStakeAmount) == -1 {
return errors.Wrap(ErrInvalidAmount, "stake amount is less than the minimum requirement")
Expand All @@ -45,8 +44,8 @@ func (p *Protocol) validateWithdrawStake(ctx context.Context, act *action.Withdr
}

func (p *Protocol) validateChangeCandidate(ctx context.Context, act *action.ChangeCandidate) error {
if !isValidCandidateName(act.Candidate()) {
return ErrInvalidCanName
if !action.IsValidCandidateName(act.Candidate()) {
return action.ErrInvalidCanName
}
return nil
}
Expand All @@ -64,8 +63,8 @@ func (p *Protocol) validateRestake(ctx context.Context, act *action.Restake) err
}

func (p *Protocol) validateCandidateRegister(ctx context.Context, act *action.CandidateRegister) error {
if !isValidCandidateName(act.Name()) {
return ErrInvalidCanName
if !action.IsValidCandidateName(act.Name()) {
return action.ErrInvalidCanName
}

if act.Amount().Cmp(p.config.RegistrationConsts.MinSelfStake) < 0 {
Expand All @@ -76,22 +75,9 @@ func (p *Protocol) validateCandidateRegister(ctx context.Context, act *action.Ca

func (p *Protocol) validateCandidateUpdate(ctx context.Context, act *action.CandidateUpdate) error {
if len(act.Name()) != 0 {
if !isValidCandidateName(act.Name()) {
return ErrInvalidCanName
if !action.IsValidCandidateName(act.Name()) {
return action.ErrInvalidCanName
}
}
return nil
}

// IsValidCandidateName check if a candidate name string is valid.
func isValidCandidateName(s string) bool {
if len(s) == 0 || len(s) > 12 {
return false
}
for _, c := range s {
if !(('a' <= c && c <= 'z') || ('0' <= c && c <= '9')) {
return false
}
}
return true
}
46 changes: 0 additions & 46 deletions action/protocol/staking/validations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,58 +11,12 @@ import (
"math/big"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/iotexproject/iotex-core/blockchain/genesis"
"github.com/iotexproject/iotex-core/test/identityset"
)

func TestIsValidCandidateName(t *testing.T) {
tests := []struct {
input string
output bool
}{
{
input: "abc",
output: true,
},
{
input: "123",
output: true,
},
{
input: "abc123abc123",
output: true,
},
{
input: "Abc123",
output: false,
},
{
input: "Abc 123",
output: false,
},
{
input: "Abc-123",
output: false,
},
{
input: "abc123abc123abc123",
output: false,
},
{
input: "",
output: false,
},
}

for _, tt := range tests {
output := isValidCandidateName(tt.input)
assert.Equal(t, tt.output, output)
}
}

func initTestProtocol(t *testing.T) (*Protocol, []*Candidate) {
require := require.New(t)
p, err := NewProtocol(nil, &BuilderConfig{
Expand Down
Loading