From 2586f16899981bc6a503da6762e0a40cf8b4c6b1 Mon Sep 17 00:00:00 2001 From: Chao Ma Date: Tue, 19 Nov 2019 00:06:13 -0800 Subject: [PATCH 1/3] add staking transaction unit test for tx copy; fix panic error --- staking/types/transaction.go | 3 +- staking/types/transaction_test.go | 90 +++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 staking/types/transaction_test.go diff --git a/staking/types/transaction.go b/staking/types/transaction.go index 49678b45a..1c823dfbc 100644 --- a/staking/types/transaction.go +++ b/staking/types/transaction.go @@ -4,7 +4,6 @@ import ( "errors" "io" "math/big" - "reflect" "sync/atomic" "github.com/ethereum/go-ethereum/common" @@ -35,7 +34,7 @@ func (d *txdata) CopyFrom(d2 *txdata) { d.AccountNonce = d2.AccountNonce d.Price = new(big.Int).Set(d2.Price) d.GasLimit = d2.GasLimit - d.StakeMsg = reflect.New(reflect.ValueOf(d2.StakeMsg).Elem().Type()).Interface() + d.StakeMsg = d2.StakeMsg d.V = new(big.Int).Set(d2.V) d.R = new(big.Int).Set(d2.R) d.S = new(big.Int).Set(d2.S) diff --git a/staking/types/transaction_test.go b/staking/types/transaction_test.go new file mode 100644 index 000000000..c05c537ac --- /dev/null +++ b/staking/types/transaction_test.go @@ -0,0 +1,90 @@ +package types + +import ( + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/harmony-one/bls/ffi/go/bls" + common2 "github.com/harmony-one/harmony/internal/common" + numeric "github.com/harmony-one/harmony/numeric" + "github.com/harmony-one/harmony/shard" +) + +// for testing purpose +var ( + testAccount = "one1pdv9lrdwl0rg5vglh4xtyrv3wjk3wsqket7zxy" + testBLSPubKey = "65f55eb3052f9e9f632b2923be594ba77c55543f5c58ee1454b9cfd658d25e06373b0f7d42a19c84768139ea294f6204" + testBLSPubKey2 = "40379eed79ed82bebfb4310894fd33b6a3f8413a78dc4d43b98d0adc9ef69f3285df05eaab9f2ce5f7227f8cb920e809" +) + +func CreateTestNewTransaction() (*StakingTransaction, error) { + dAddr, _ := common2.Bech32ToAddress(testAccount) + + stakePayloadMaker := func() (Directive, interface{}) { + p := &bls.PublicKey{} + p.DeserializeHexStr(testBLSPubKey) + pub := shard.BlsPublicKey{} + pub.FromLibBLSPublicKey(p) + + ra, _ := numeric.NewDecFromStr("0.7") + maxRate, _ := numeric.NewDecFromStr("1") + maxChangeRate, _ := numeric.NewDecFromStr("0.5") + return DirectiveCreateValidator, CreateValidator{ + Description: &Description{ + Name: "SuperHero", + Identity: "YouWouldNotKnow", + Website: "Secret Website", + SecurityContact: "LicenseToKill", + Details: "blah blah blah", + }, + CommissionRates: CommissionRates{ + Rate: ra, + MaxRate: maxRate, + MaxChangeRate: maxChangeRate, + }, + MinSelfDelegation: big.NewInt(10), + MaxTotalDelegation: big.NewInt(3000), + ValidatorAddress: common.Address(dAddr), + SlotPubKeys: []shard.BlsPublicKey{pub}, + Amount: big.NewInt(100), + } + } + + gasPrice := big.NewInt(1) + return NewStakingTransaction(0, 600000, gasPrice, stakePayloadMaker) +} + +func TestTransactionCopy(t *testing.T) { + tx1, err := CreateTestNewTransaction() + if err != nil { + t.Errorf("cannot create new staking transaction, %v\n", err) + } + tx2 := tx1.Copy() + + cv1 := tx1.data.StakeMsg.(CreateValidator) + cv1.Amount = big.NewInt(20) + cv1.Description.Name = "NewName" + + p := &bls.PublicKey{} + p.DeserializeHexStr(testBLSPubKey2) + pub := shard.BlsPublicKey{} + pub.FromLibBLSPublicKey(p) + cv1.SlotPubKeys = append(cv1.SlotPubKeys, pub) + + tx1.data.StakeMsg = cv1 + + cv2 := tx2.data.StakeMsg.(CreateValidator) + + if cv1.Amount.Cmp(cv2.Amount) == 0 { + t.Errorf("Amount should not be equal") + } + + if len(cv1.SlotPubKeys) == len(cv2.SlotPubKeys) { + t.Errorf("SlotPubKeys should not be equal length") + } + + if len(cv1.Description.Name) == len(cv2.Description.Name) { + t.Errorf("Description name should not be the same") + } +} From 4a185825a5effa9af7bbe07e881e21bb6b565c38 Mon Sep 17 00:00:00 2001 From: Chao Ma Date: Tue, 19 Nov 2019 16:34:16 -0800 Subject: [PATCH 2/3] add StakeMsg interface to deep copy --- staking/types/messages.go | 65 ++++++++++++++++++++++++++++++++++++ staking/types/transaction.go | 4 +-- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/staking/types/messages.go b/staking/types/messages.go index 8794b977c..ffd20ed62 100644 --- a/staking/types/messages.go +++ b/staking/types/messages.go @@ -14,6 +14,12 @@ import ( // Directive says what kind of payload follows type Directive byte +// StakeMsg defines the interface of Stake Message +type StakeMsg interface { + Type() Directive + Copy() StakeMsg +} + const ( // DirectiveCreateValidator ... DirectiveCreateValidator Directive = iota @@ -86,3 +92,62 @@ type Undelegate struct { type CollectRewards struct { DelegatorAddress common.Address `json:"delegator_address" yaml:"delegator_address"` } + +// Type of CreateValidator +func (v CreateValidator) Type() Directive { + return DirectiveCreateValidator +} + +// Type of EditValidator +func (v EditValidator) Type() Directive { + return DirectiveEditValidator +} + +// Type of Delegate +func (v Delegate) Type() Directive { + return DirectiveCreateValidator +} + +// Type of Undelegate +func (v Undelegate) Type() Directive { + return DirectiveUndelegate +} + +// Type of CollectRewards +func (v CollectRewards) Type() Directive { + return DirectiveCollectRewards +} + +// Copy deep copy of the interface +func (v CreateValidator) Copy() StakeMsg { + v1 := v + desc := *v.Description + v1.Description = &desc + return v1 +} + +// Copy deep copy of the interface +func (v EditValidator) Copy() StakeMsg { + v1 := v + desc := *v.Description + v1.Description = &desc + return v1 +} + +// Copy deep copy of the interface +func (v Delegate) Copy() StakeMsg { + v1 := v + return v1 +} + +// Copy deep copy of the interface +func (v Undelegate) Copy() StakeMsg { + v1 := v + return v1 +} + +// Copy deep copy of the interface +func (v CollectRewards) Copy() StakeMsg { + v1 := v + return v1 +} diff --git a/staking/types/transaction.go b/staking/types/transaction.go index 1c823dfbc..d4ba23775 100644 --- a/staking/types/transaction.go +++ b/staking/types/transaction.go @@ -30,11 +30,11 @@ type txdata struct { } func (d *txdata) CopyFrom(d2 *txdata) { - d.Directive = d2.Directive d.AccountNonce = d2.AccountNonce d.Price = new(big.Int).Set(d2.Price) d.GasLimit = d2.GasLimit - d.StakeMsg = d2.StakeMsg + // TODO: add code to protect crashing + d.StakeMsg = d2.StakeMsg.(StakeMsg).Copy() d.V = new(big.Int).Set(d2.V) d.R = new(big.Int).Set(d2.R) d.S = new(big.Int).Set(d2.S) From 8455c693c8ab5a8899485d0e0f843b04b4ce4bbd Mon Sep 17 00:00:00 2001 From: Chao Ma Date: Tue, 19 Nov 2019 16:42:24 -0800 Subject: [PATCH 3/3] add unit test for commission rate field --- staking/types/messages.go | 2 +- staking/types/transaction_test.go | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/staking/types/messages.go b/staking/types/messages.go index ffd20ed62..d17d808af 100644 --- a/staking/types/messages.go +++ b/staking/types/messages.go @@ -105,7 +105,7 @@ func (v EditValidator) Type() Directive { // Type of Delegate func (v Delegate) Type() Directive { - return DirectiveCreateValidator + return DirectiveDelegate } // Type of Undelegate diff --git a/staking/types/transaction_test.go b/staking/types/transaction_test.go index c05c537ac..7abd111bc 100644 --- a/staking/types/transaction_test.go +++ b/staking/types/transaction_test.go @@ -1,6 +1,7 @@ package types import ( + "fmt" "math/big" "testing" @@ -63,8 +64,12 @@ func TestTransactionCopy(t *testing.T) { tx2 := tx1.Copy() cv1 := tx1.data.StakeMsg.(CreateValidator) + + // modify cv1 fields cv1.Amount = big.NewInt(20) cv1.Description.Name = "NewName" + newRate, _ := numeric.NewDecFromStr("0.5") + cv1.CommissionRates.Rate = newRate p := &bls.PublicKey{} p.DeserializeHexStr(testBLSPubKey2) @@ -87,4 +92,13 @@ func TestTransactionCopy(t *testing.T) { if len(cv1.Description.Name) == len(cv2.Description.Name) { t.Errorf("Description name should not be the same") } + + if cv1.CommissionRates.Rate.Equal(cv2.CommissionRates.Rate) { + t.Errorf("CommissionRate should not be equal") + } + + fmt.Println("cv1", cv1) + fmt.Println("cv2", cv2) + fmt.Println("cv1", cv1.Description) + fmt.Println("cv2", cv2.Description) }