Skip to content

Handle builder validation errors #4714

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Handle builder validation errors #4714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 12 additions & 9 deletions action/action_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ func TestActionProtoAndVerify(t *testing.T) {
v := NewExecution("", big.NewInt(10), data)
t.Run("no error", func(t *testing.T) {
bd := &EnvelopeBuilder{}
elp := bd.SetGasPrice(big.NewInt(10)).
elp, err := bd.SetGasPrice(big.NewInt(10)).
SetGasLimit(uint64(100000)).
SetAction(v).Build()

require.NoError(err)
selp, err := Sign(elp, identityset.PrivateKey(28))
require.NoError(err)
require.Equal(65, len(selp.SrcPubkey().Bytes()))
Expand All @@ -45,10 +45,10 @@ func TestActionProtoAndVerify(t *testing.T) {
})
t.Run("empty public key", func(t *testing.T) {
bd := &EnvelopeBuilder{}
elp := bd.SetGasPrice(big.NewInt(10)).
elp, err := bd.SetGasPrice(big.NewInt(10)).
SetGasLimit(uint64(100000)).
SetAction(v).Build()

require.NoError(err)
selp, err := Sign(elp, identityset.PrivateKey(28))
require.NoError(err)

Expand All @@ -58,10 +58,10 @@ func TestActionProtoAndVerify(t *testing.T) {
})
t.Run("invalid signature", func(t *testing.T) {
bd := &EnvelopeBuilder{}
elp := bd.SetGasPrice(big.NewInt(10)).
elp, err := bd.SetGasPrice(big.NewInt(10)).
SetGasLimit(uint64(100000)).
SetAction(v).Build()

require.NoError(err)
selp, err := Sign(elp, identityset.PrivateKey(28))
require.NoError(err)
selp.signature = []byte("invalid signature")
Expand Down Expand Up @@ -107,19 +107,22 @@ func TestIsSystemAction(t *testing.T) {
require := require.New(t)
builder := EnvelopeBuilder{}
actClaimFromRewarding := ClaimFromRewardingFund{}
act := builder.SetAction(&actClaimFromRewarding).Build()
act, err := builder.SetAction(&actClaimFromRewarding).Build()
require.NoError(err)
sel, err := Sign(act, identityset.PrivateKey(1))
require.NoError(err)
require.False(IsSystemAction(sel))

actGrantReward := NewGrantReward(EpochReward, 1)
act = builder.SetAction(actGrantReward).Build()
act, err = builder.SetAction(actGrantReward).Build()
require.NoError(err)
sel, err = Sign(act, identityset.PrivateKey(1))
require.NoError(err)
require.True(IsSystemAction(sel))

actPollResult := NewPutPollResult(1, nil)
act = builder.SetAction(actPollResult).Build()
act, err = builder.SetAction(actPollResult).Build()
require.NoError(err)
sel, err = Sign(act, identityset.PrivateKey(1))
require.NoError(err)
require.True(IsSystemAction(sel))
Expand Down
18 changes: 9 additions & 9 deletions action/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ func (b *EnvelopeBuilder) SetBlobTxData(
}

// Build builds a new action.
func (b *EnvelopeBuilder) Build() Envelope {
func (b *EnvelopeBuilder) Build() (Envelope, error) {
return b.build()
}

func (b *EnvelopeBuilder) build() Envelope {
func (b *EnvelopeBuilder) build() (Envelope, error) {
if b.ab.gasPrice == nil {
b.ab.gasPrice = big.NewInt(0)
}
Expand All @@ -174,13 +174,13 @@ func (b *EnvelopeBuilder) build() Envelope {
b.ab.version = version.ProtocolVersion
}
if err := b.ab.validateTx(); err != nil {
panic(err.Error())
return nil, err
}
if b.elp.payload == nil {
panic("cannot build Envelope w/o a valid payload")
return nil, errors.New("cannot build Envelope w/o a valid payload")
}
b.elp.common = b.ab.convertToTx()
return &b.elp
return &b.elp, nil
}

// BuildTransfer loads transfer action into envelope
Expand All @@ -192,7 +192,7 @@ func (b *EnvelopeBuilder) BuildTransfer(tx *types.Transaction) (Envelope, error)
return nil, err
}
b.elp.payload = NewTransfer(tx.Value(), getRecipientAddr(tx.To()), tx.Data())
return b.build(), nil
return b.build()
}

func (b *EnvelopeBuilder) setEnvelopeCommonFields(tx *types.Transaction) error {
Expand Down Expand Up @@ -248,7 +248,7 @@ func (b *EnvelopeBuilder) BuildExecution(tx *types.Transaction) (Envelope, error
return nil, err
}
b.elp.payload = NewExecution(getRecipientAddr(tx.To()), tx.Value(), tx.Data())
return b.build(), nil
return b.build()
}

// BuildStakingAction loads staking action into envelope from abi-encoded data
Expand All @@ -264,7 +264,7 @@ func (b *EnvelopeBuilder) BuildStakingAction(tx *types.Transaction) (Envelope, e
return nil, err
}
b.elp.payload = act
return b.build(), nil
return b.build()
}

// BuildRewardingAction loads rewarding action into envelope from abi-encoded data
Expand All @@ -280,7 +280,7 @@ func (b *EnvelopeBuilder) BuildRewardingAction(tx *types.Transaction) (Envelope,
return nil, err
}
b.elp.payload = act
return b.build(), nil
return b.build()
}

func newStakingActionFromABIBinary(data []byte, value *big.Int) (actionPayload, error) {
Expand Down
20 changes: 19 additions & 1 deletion action/builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ func TestActionBuilder(t *testing.T) {
r.Equal(big.NewInt(10004), act.GasPrice())
}

func TestEnvelopeBuilderBuildError(t *testing.T) {
r := require.New(t)

b := &EnvelopeBuilder{}
// build without payload
_, err := b.Build()
r.Error(err)

// invalid tx type
_, err = b.SetTxType(255).SetAction(&Transfer{}).Build()
r.Error(err)

// BuildTransfer with nil recipient should return error, not panic
_, err = (&EnvelopeBuilder{}).BuildTransfer(types.NewTx(&types.LegacyTx{Nonce: 1, GasPrice: big.NewInt(1), Gas: 1}))
r.Error(err)
}

func TestBuildRewardingAction(t *testing.T) {
r := require.New(t)

Expand Down Expand Up @@ -159,8 +176,9 @@ func TestEthTxUtils(t *testing.T) {
addr, err := address.FromHex("0xA576C141e5659137ddDa4223d209d4744b2106BE")
r.NoError(err)
act := NewClaimFromRewardingFund(big.NewInt(1), addr, []byte("any"))
elp := (&EnvelopeBuilder{}).SetNonce(100).SetGasLimit(21000).
elp, err := (&EnvelopeBuilder{}).SetNonce(100).SetGasLimit(21000).
SetGasPrice(big.NewInt(101)).SetAction(act).Build()
r.NoError(err)
tx, err := elp.ToEthTx(chainID, iotextypes.Encoding_ETHEREUM_EIP155)
r.NoError(err)

Expand Down
3 changes: 2 additions & 1 deletion action/candidate_transfer_ownership_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ func TestCandidateTransferOwnership(t *testing.T) {
if err != nil {
continue
}
elp := (&EnvelopeBuilder{}).SetNonce(test.nonce).SetGasLimit(test.gasLimit).
elp, err := (&EnvelopeBuilder{}).SetNonce(test.nonce).SetGasLimit(test.gasLimit).
SetGasPrice(test.gasPrice).SetAction(cr).Build()
require.NoError(err)
err = elp.SanityCheck()
require.Equal(test.sanityCheck, errors.Cause(err))
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion action/candidateregister_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ func TestCandidateRegister(t *testing.T) {
if err != nil {
continue
}
elp := (&EnvelopeBuilder{}).SetNonce(test.Nonce).SetGasLimit(test.GasLimit).
elp, err := (&EnvelopeBuilder{}).SetNonce(test.Nonce).SetGasLimit(test.GasLimit).
SetGasPrice(test.GasPrice).SetAction(cr).Build()
require.NoError(err)
err = elp.SanityCheck()
require.Equal(test.SanityCheck, errors.Cause(err))
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion action/candidateupdate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ func TestCandidateUpdate(t *testing.T) {
require := require.New(t)
cu, err := NewCandidateUpdate(_cuName, _cuOperatorAddrStr, _cuRewardAddrStr)
require.NoError(err)
elp := (&EnvelopeBuilder{}).SetNonce(_cuNonce).SetGasLimit(_cuGasLimit).
elp, err := (&EnvelopeBuilder{}).SetNonce(_cuNonce).SetGasLimit(_cuGasLimit).
SetGasPrice(_cuGasPrice).SetAction(cu).Build()
require.NoError(err)
t.Run("proto", func(t *testing.T) {
ser := cu.Serialize()
require.Equal("0a04746573741229696f31636c36726c32657635646661393838716d677a673278346866617a6d7039766e326736366e671a29696f316a757678356730363365753474733833326e756b7034766763776b32676e6335637539617964", hex.EncodeToString(ser))
Expand Down
6 changes: 4 additions & 2 deletions action/claimreward_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ func TestClaimReward(t *testing.T) {
t.Run("cost", func(t *testing.T) {
rc := &ClaimFromRewardingFund{}
builder := (&EnvelopeBuilder{}).SetGasPrice(big.NewInt(1000000000000))
elp := builder.SetAction(rc).Build()
elp, err := builder.SetAction(rc).Build()
r.NoError(err)
cost, err := elp.Cost()
r.NoError(err)
r.Equal("10000000000000000", cost.String())
Expand Down Expand Up @@ -193,8 +194,9 @@ func TestClaimFromRewardingFund(t *testing.T) {
r.NoError(c.SanityCheck())
intrinsicGas, err := c.IntrinsicGas()
r.NoError(err)
elp := (&EnvelopeBuilder{}).SetGasPrice(big.NewInt(10)).
elp, err := (&EnvelopeBuilder{}).SetGasPrice(big.NewInt(10)).
SetAction(c).Build()
r.NoError(err)
cost, err := elp.Cost()
r.NoError(err)
gas := new(big.Int).SetUint64(intrinsicGas)
Expand Down
3 changes: 2 additions & 1 deletion action/depositreward_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ func TestDepositRewardSerialize(t *testing.T) {
})
t.Run("cost", func(t *testing.T) {
rp := &DepositToRewardingFund{amount: big.NewInt(100)}
elp := (&EnvelopeBuilder{}).SetGasPrice(_defaultGasPrice).
elp, err := (&EnvelopeBuilder{}).SetGasPrice(_defaultGasPrice).
SetAction(rp).Build()
r.NoError(err)
cost, err := elp.Cost()
r.NoError(err)
r.EqualValues("10000000000000100", cost.String())
Expand Down
8 changes: 6 additions & 2 deletions action/envelope_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ func TestEnvelope_Actions(t *testing.T) {
if txtype == BlobTxType {
bd.SetBlobTxData(uint256.NewInt(1), []common.Hash{}, nil)
}
elp := bd.SetNonce(1).SetGasLimit(_gasLimit).SetGasPrice(_gasPrice).
elp, err := bd.SetNonce(1).SetGasLimit(_gasLimit).SetGasPrice(_gasPrice).
SetTxType(txtype).SetAction(test).SetChainID(1).Build()
require.NoError(err)
evlp := envelope{}
require.NoError(evlp.LoadProto(elp.Proto()))
require.Equal(elp.TxType(), evlp.TxType())
Expand All @@ -134,9 +135,12 @@ func createEnvelope(chainID uint32) (Envelope, *Transfer) {
tsf := NewTransfer(unit.ConvertIotxToRau(1000+int64(10)),
identityset.Address(10%identityset.Size()).String(),
nil)
evlp := (&EnvelopeBuilder{}).SetAction(tsf).SetGasLimit(20010).
evlp, err := (&EnvelopeBuilder{}).SetAction(tsf).SetGasLimit(20010).
SetGasPrice(unit.ConvertIotxToRau(11)).SetNonce(10).
SetVersion(1).SetChainID(chainID).Build()
if err != nil {
panic(err)
}
return evlp, tsf
}

Expand Down
9 changes: 6 additions & 3 deletions action/execution_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ func TestExecutionSignVerify(t *testing.T) {
require.EqualValues(66, ex.Size())

bd := &EnvelopeBuilder{}
eb := bd.SetNonce(2).SetGasLimit(100000).SetGasPrice(big.NewInt(10)).
eb, err := bd.SetNonce(2).SetGasLimit(100000).SetGasPrice(big.NewInt(10)).
SetAction(ex).Build()
require.NoError(err)
elp, ok := eb.(*envelope)
require.True(ok)
require.EqualValues(87, eb.Size())
Expand Down Expand Up @@ -79,7 +80,8 @@ func TestExecutionSanityCheck(t *testing.T) {

t.Run("Negative gas price", func(t *testing.T) {
ex := NewExecution(identityset.Address(29).String(), big.NewInt(100), []byte{})
elp := (&EnvelopeBuilder{}).SetGasPrice(big.NewInt(-1)).SetAction(ex).Build()
elp, err := (&EnvelopeBuilder{}).SetGasPrice(big.NewInt(-1)).SetAction(ex).Build()
require.NoError(err)
require.Equal(ErrNegativeValue, errors.Cause(elp.SanityCheck()))
})
}
Expand Down Expand Up @@ -125,8 +127,9 @@ func TestExecutionAccessList(t *testing.T) {
identityset.Address(29).String(),
big.NewInt(20),
[]byte("test"))
elp := (&EnvelopeBuilder{}).SetTxType(AccessListTxType).SetNonce(1).SetAccessList(v.list).
elp, err := (&EnvelopeBuilder{}).SetTxType(AccessListTxType).SetNonce(1).SetAccessList(v.list).
SetGasPrice(big.NewInt(1000000)).SetGasLimit(100).SetAction(ex).Build()
require.NoError(err)
require.NoError(ex1.LoadProto(ex.Proto()))
require.Equal(ex, ex1)
gas, err := elp.IntrinsicGas()
Expand Down
3 changes: 2 additions & 1 deletion action/grantreward_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ func TestGrandReward(t *testing.T) {
intrinsicGas, err := g.IntrinsicGas()
require.NoError(err)
require.Zero(intrinsicGas)
elp := (&EnvelopeBuilder{}).SetGasPrice(_defaultGasPrice).
elp, err := (&EnvelopeBuilder{}).SetGasPrice(_defaultGasPrice).
SetAction(g).Build()
require.NoError(err)
cost, err := elp.Cost()
require.NoError(err)
require.Equal(big.NewInt(0), cost)
Expand Down
3 changes: 2 additions & 1 deletion action/putpollresult_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ func TestPutPollResult(t *testing.T) {
igas, err := r.IntrinsicGas()
assert.NoError(t, err)
assert.Zero(t, igas)
elp := (&EnvelopeBuilder{}).SetNonce(1).SetGasLimit(uint64(100000)).
elp, err := (&EnvelopeBuilder{}).SetNonce(1).SetGasLimit(uint64(100000)).
SetAction(r).Build()
assert.NoError(t, err)
cost, err := elp.Cost()
assert.NoError(t, err)
assert.Equal(t, big.NewInt(0), cost)
Expand Down
8 changes: 6 additions & 2 deletions action/rlp_tx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ func TestGenerateRlp(t *testing.T) {
data: _signByte,
}, _validSig, "", hash.BytesToHash256(MustNoErrorV(hex.DecodeString("fee3db88ee7d7defa9eded672d08fc8641f760f3a11d404a53276ad6f412b8a5")))},
} {
elp := builder.SetAction(v.act).Build()
elp, err := builder.SetAction(v.act).Build()
require.NoError(err)
tx, err := elp.ToEthTx(0, iotextypes.Encoding_ETHEREUM_EIP155)
if err != nil {
require.Contains(err.Error(), v.err)
Expand Down Expand Up @@ -826,7 +827,10 @@ func TestEthTxDecodeVerifyV2(t *testing.T) {
}
// build eth tx from test case
var (
elp = elpbuilder.SetAction(tt.action).Build()
elp, err = elpbuilder.SetAction(tt.action).Build()
)
require.NoError(t, err)
var (
tx = MustNoErrorV(elp.ToEthTx(chainID, tt.encoding))
signer = MustNoErrorV(NewEthSigner(tt.encoding, chainID))
signature = MustNoErrorV(sk.Sign(tx.Hash().Bytes()))
Expand Down
6 changes: 4 additions & 2 deletions action/sealedenvelope_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ func TestSealedEnvelope_InvalidType(t *testing.T) {
candidates := state.CandidateList{}
r := NewPutPollResult(10001, candidates)

elp := (&EnvelopeBuilder{}).SetNonce(1).SetAction(r).
elp, err := (&EnvelopeBuilder{}).SetNonce(1).SetAction(r).
SetGasLimit(100000).Build()
require.NoError(err)
selp := FakeSeal(elp, identityset.PrivateKey(27).PublicKey())
selp.encoding = iotextypes.Encoding_ETHEREUM_EIP155
hash1, err := selp.envelopeHash()
Expand Down Expand Up @@ -102,9 +103,10 @@ func TestSealedEnvelope_Actions(t *testing.T) {

for _, test := range tests {
bd := &EnvelopeBuilder{}
elp := bd.SetNonce(1).
elp, err := bd.SetNonce(1).
SetAction(test).
SetGasLimit(100000).Build()
require.NoError(err)
selp := FakeSeal(elp, identityset.PrivateKey(27).PublicKey())
rlp, err := selp.ToEthTx()
require.NoError(err)
Expand Down
Loading
Loading