IOTEX-270 Improve rewarding protocol (iotexproject#593)

Author: zjshen14

action/protocol/execution/evm/evm.go

@@ -11,15 +11,14 @@ import (
 	"math"
 	"math/big"
 
-	"github.com/iotexproject/iotex-core/action/protocol/rewarding"
-
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/pkg/errors"
 
 	"github.com/iotexproject/iotex-core/action"
 	"github.com/iotexproject/iotex-core/action/protocol"
+	"github.com/iotexproject/iotex-core/action/protocol/rewarding"
 	"github.com/iotexproject/iotex-core/address"
 	"github.com/iotexproject/iotex-core/pkg/log"
 )
@@ -38,13 +37,6 @@ func MakeTransfer(db vm.StateDB, fromHash, toHash common.Address, amount *big.In
 	db.AddBalance(toHash, amount)
 }
 
-const (
-	// FailureStatus is the status that contract execution failed
-	FailureStatus = uint64(0)
-	// SuccessStatus is the status that contract execution success
-	SuccessStatus = uint64(1)
-)
-
 // Params is the context and parameters
 type Params struct {
 	context            vm.Context
@@ -161,9 +153,9 @@ func ExecuteContract(
 		ContractAddress: contractAddress,
 	}
 	if err != nil {
-		receipt.Status = FailureStatus
+		receipt.Status = action.FailureReceiptStatus
 	} else {
-		receipt.Status = SuccessStatus
+		receipt.Status = action.SuccessReceiptStatus
 	}
 	if remainingGas > 0 {
 		*raCtx.GasLimit += remainingGas

action/protocol/rewarding/admin.go

@@ -22,18 +22,18 @@ import (
 // admin stores the admin data of the rewarding protocol
 type admin struct {
 	admin                      address.Address
-	BlockReward                *big.Int
-	EpochReward                *big.Int
-	NumDelegatesForEpochReward uint64
+	blockReward                *big.Int
+	epochReward                *big.Int
+	numDelegatesForEpochReward uint64
 }
 
 // Serialize serializes admin state into bytes
 func (a admin) Serialize() ([]byte, error) {
 	gen := rewardingpb.Admin{
 		Admin:                      a.admin.Bytes(),
-		BlockReward:                a.BlockReward.Bytes(),
-		EpochReward:                a.EpochReward.Bytes(),
-		NumDelegatesForEpochReward: a.NumDelegatesForEpochReward,
+		BlockReward:                a.blockReward.Bytes(),
+		EpochReward:                a.epochReward.Bytes(),
+		NumDelegatesForEpochReward: a.numDelegatesForEpochReward,
 	}
 	return proto.Marshal(&gen)
 }
@@ -48,9 +48,9 @@ func (a *admin) Deserialize(data []byte) error {
 	if a.admin, err = address.FromBytes(gen.Admin); err != nil {
 		return err
 	}
-	a.BlockReward = big.NewInt(0).SetBytes(gen.BlockReward)
-	a.EpochReward = big.NewInt(0).SetBytes(gen.EpochReward)
-	a.NumDelegatesForEpochReward = gen.NumDelegatesForEpochReward
+	a.blockReward = big.NewInt(0).SetBytes(gen.BlockReward)
+	a.epochReward = big.NewInt(0).SetBytes(gen.EpochReward)
+	a.numDelegatesForEpochReward = gen.NumDelegatesForEpochReward
 	return nil
 }
 
@@ -79,9 +79,9 @@ func (p *Protocol) Initialize(
 		adminKey,
 		&admin{
 			admin:                      adminAddr,
-			BlockReward:                blockReward,
-			EpochReward:                epochReward,
-			NumDelegatesForEpochReward: numDelegatesForEpochReward,
+			blockReward:                blockReward,
+			epochReward:                epochReward,
+			numDelegatesForEpochReward: numDelegatesForEpochReward,
 		},
 	); err != nil {
 		return err
@@ -135,7 +135,7 @@ func (p *Protocol) BlockReward(
 	if err := p.state(sm, adminKey, &a); err != nil {
 		return nil, err
 	}
-	return a.BlockReward, nil
+	return a.blockReward, nil
 }
 
 // SetBlockReward sets the block reward amount for the block rewarding. Only the current admin could make this change
@@ -156,7 +156,7 @@ func (p *Protocol) EpochReward(
 	if err := p.state(sm, adminKey, &a); err != nil {
 		return nil, err
 	}
-	return a.EpochReward, nil
+	return a.epochReward, nil
 }
 
 // SetEpochReward sets the epoch reward amount shared by all beneficiaries in an epoch. Only the current admin could
@@ -178,7 +178,7 @@ func (p *Protocol) NumDelegatesForEpochReward(
 	if err := p.state(sm, adminKey, &a); err != nil {
 		return 0, err
 	}
-	return a.NumDelegatesForEpochReward, nil
+	return a.numDelegatesForEpochReward, nil
 }
 
 // SetNumDelegatesForEpochReward sets the number of candidates sharing an epoch reward
@@ -195,7 +195,7 @@ func (p *Protocol) SetNumDelegatesForEpochReward(
 	if err := p.state(sm, adminKey, &a); err != nil {
 		return err
 	}
-	a.NumDelegatesForEpochReward = num
+	a.numDelegatesForEpochReward = num
 	return p.putState(sm, adminKey, &a)
 }
 
@@ -242,9 +242,9 @@ func (p *Protocol) setReward(
 		return err
 	}
 	if blockLevel {
-		a.BlockReward = amount
+		a.blockReward = amount
 	} else {
-		a.EpochReward = amount
+		a.epochReward = amount
 	}
 	return p.putState(sm, adminKey, &a)
 }

action/protocol/rewarding/fund.go

@@ -53,10 +53,7 @@ func (p *Protocol) Deposit(
 	sm protocol.StateManager,
 	amount *big.Int,
 ) error {
-	raCtx, ok := protocol.GetRunActionsCtx(ctx)
-	if !ok {
-		log.S().Panic("Miss run action context")
-	}
+	raCtx := protocol.MustGetRunActionsCtx(ctx)
 	if err := p.assertEnoughBalance(raCtx, sm, amount); err != nil {
 		return err
 	}
@@ -118,6 +115,10 @@ func (p *Protocol) assertEnoughBalance(
 
 // DepositGas deposits gas into the rewarding fund
 func DepositGas(ctx context.Context, sm protocol.StateManager, amount *big.Int, registry *protocol.Registry) error {
+	// If the gas fee is 0, return immediately
+	if amount.Cmp(big.NewInt(0)) == 0 {
+		return nil
+	}
 	// TODO: we bypass the gas deposit for the actions in genesis block. Later we should remove this after we remove
 	// genesis actions
 	raCtx := protocol.MustGetRunActionsCtx(ctx)

action/protocol/rewarding/protocol.go

@@ -75,38 +75,44 @@ func (p *Protocol) Handle(
 	case *action.SetReward:
 		switch act.RewardType() {
 		case action.BlockReward:
+			si := sm.Snapshot()
 			if err := p.SetBlockReward(ctx, sm, act.Amount()); err != nil {
-				return p.settleAction(ctx, sm, 1), nil
+				return p.settleAction(ctx, sm, action.FailureReceiptStatus, si)
 			}
-			return p.settleAction(ctx, sm, 0), nil
+			return p.settleAction(ctx, sm, action.SuccessReceiptStatus, si)
 		case action.EpochReward:
+			si := sm.Snapshot()
 			if err := p.SetEpochReward(ctx, sm, act.Amount()); err != nil {
-				return p.settleAction(ctx, sm, 1), nil
+				return p.settleAction(ctx, sm, action.FailureReceiptStatus, si)
 			}
-			return p.settleAction(ctx, sm, 0), nil
+			return p.settleAction(ctx, sm, action.SuccessReceiptStatus, si)
 		}
 	case *action.DepositToRewardingFund:
+		si := sm.Snapshot()
 		if err := p.Deposit(ctx, sm, act.Amount()); err != nil {
-			return p.settleAction(ctx, sm, 1), nil
+			return p.settleAction(ctx, sm, action.FailureReceiptStatus, si)
 		}
-		return p.settleAction(ctx, sm, 0), nil
+		return p.settleAction(ctx, sm, action.SuccessReceiptStatus, si)
 	case *action.ClaimFromRewardingFund:
+		si := sm.Snapshot()
 		if err := p.Claim(ctx, sm, act.Amount()); err != nil {
-			return p.settleAction(ctx, sm, 1), nil
+			return p.settleAction(ctx, sm, action.FailureReceiptStatus, si)
 		}
-		return p.settleAction(ctx, sm, 0), nil
+		return p.settleAction(ctx, sm, action.SuccessReceiptStatus, si)
 	case *action.GrantReward:
 		switch act.RewardType() {
 		case action.BlockReward:
+			si := sm.Snapshot()
 			if err := p.GrantBlockReward(ctx, sm); err != nil {
-				return p.settleAction(ctx, sm, 1), nil
+				return p.settleAction(ctx, sm, action.FailureReceiptStatus, si)
 			}
-			return p.settleAction(ctx, sm, 0), nil
+			return p.settleAction(ctx, sm, action.SuccessReceiptStatus, si)
 		case action.EpochReward:
+			si := sm.Snapshot()
 			if err := p.GrantEpochReward(ctx, sm); err != nil {
-				return p.settleAction(ctx, sm, 1), nil
+				return p.settleAction(ctx, sm, action.FailureReceiptStatus, si)
 			}
-			return p.settleAction(ctx, sm, 0), nil
+			return p.settleAction(ctx, sm, action.SuccessReceiptStatus, si)
 		}
 	}
 	return nil, nil
@@ -166,16 +172,22 @@ func (p *Protocol) settleAction(
 	ctx context.Context,
 	sm protocol.StateManager,
 	status uint64,
-) *action.Receipt {
+	si int,
+) (*action.Receipt, error) {
 	raCtx := protocol.MustGetRunActionsCtx(ctx)
+	if status == action.FailureReceiptStatus {
+		if err := sm.Revert(si); err != nil {
+			return nil, err
+		}
+	}
 	gasFee := big.NewInt(0).Mul(raCtx.GasPrice, big.NewInt(0).SetUint64(raCtx.IntrinsicGas))
 	if err := DepositGas(ctx, sm, gasFee, raCtx.Registry); err != nil {
-		p.createReceipt(1, raCtx.ActionHash, raCtx.IntrinsicGas)
+		return nil, err
 	}
 	if err := p.increaseNonce(sm, raCtx.Caller, raCtx.Nonce); err != nil {
-		return p.createReceipt(1, raCtx.ActionHash, raCtx.IntrinsicGas)
+		return nil, err
 	}
-	return p.createReceipt(status, raCtx.ActionHash, raCtx.IntrinsicGas)
+	return p.createReceipt(status, raCtx.ActionHash, raCtx.IntrinsicGas), nil
 }
 
 func (p *Protocol) increaseNonce(sm protocol.StateManager, addr address.Address, nonce uint64) error {
@@ -194,7 +206,7 @@ func (p *Protocol) createReceipt(status uint64, actHash hash.Hash256, gasConsume
 	// TODO: need to review the fields
 	return &action.Receipt{
 		ReturnValue:     nil,
-		Status:          0,
+		Status:          status,
 		ActHash:         actHash,
 		GasConsumed:     gasConsumed,
 		ContractAddress: p.addr.String(),

action/protocol/rewarding/protocol_test.go

@@ -11,6 +11,10 @@ import (
 	"math/big"
 	"testing"
 
+	"github.com/iotexproject/iotex-core/action"
+
+	"github.com/iotexproject/iotex-core/test/identityset"
+
 	"github.com/iotexproject/iotex-core/blockchain/genesis"
 
 	"github.com/golang/mock/gomock"
@@ -57,6 +61,10 @@ func testProtocol(t *testing.T, test func(*testing.T, context.Context, factory.F
 			Address: testaddress.Addrinfo["charlie"].String(),
 			Votes:   unit.ConvertIotxToRau(1000000),
 		},
+		{
+			Address: testaddress.Addrinfo["delta"].String(),
+			Votes:   unit.ConvertIotxToRau(500000),
+		},
 	}, nil).AnyTimes()
 	p := NewProtocol(chain, genesis.Default.NumDelegates, genesis.Default.NumSubEpochs)
 
@@ -69,7 +77,7 @@ func testProtocol(t *testing.T, test func(*testing.T, context.Context, factory.F
 	)
 	ws, err := stateDB.NewWorkingSet()
 	require.NoError(t, err)
-	require.NoError(t, p.Initialize(ctx, ws, testaddress.Addrinfo["alfa"], big.NewInt(0), big.NewInt(10), big.NewInt(100), 10))
+	require.NoError(t, p.Initialize(ctx, ws, testaddress.Addrinfo["alfa"], big.NewInt(0), big.NewInt(10), big.NewInt(100), 4))
 	require.NoError(t, stateDB.Commit(ws))
 
 	ctx = protocol.WithRunActionsCtx(
@@ -108,3 +116,66 @@ func testProtocol(t *testing.T, test func(*testing.T, context.Context, factory.F
 
 	test(t, ctx, stateDB, p)
 }
+
+func TestProtocol_Handle(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	cfg := config.Default
+	stateDB, err := factory.NewStateDB(cfg, factory.InMemStateDBOption())
+	require.NoError(t, err)
+	require.NoError(t, stateDB.Start(context.Background()))
+	defer func() {
+		require.NoError(t, stateDB.Stop(context.Background()))
+	}()
+	chain := mock_chainmanager.NewMockChainManager(ctrl)
+	p := NewProtocol(chain, genesis.Default.NumDelegates, genesis.Default.NumSubEpochs)
+
+	ctx := protocol.WithRunActionsCtx(
+		context.Background(),
+		protocol.RunActionsCtx{
+			BlockHeight: 0,
+		},
+	)
+	ws, err := stateDB.NewWorkingSet()
+	require.NoError(t, err)
+	require.NoError(t, p.Initialize(
+		ctx,
+		ws,
+		identityset.Address(0),
+		big.NewInt(1000000),
+		big.NewInt(10),
+		big.NewInt(100),
+		10,
+	))
+	require.NoError(t, stateDB.Commit(ws))
+
+	ws, err = stateDB.NewWorkingSet()
+	require.NoError(t, err)
+	gb := action.GrantRewardBuilder{}
+	grant := gb.SetRewardType(action.BlockReward).Build()
+	eb := action.EnvelopeBuilder{}
+	e := eb.SetNonce(0).
+		SetGasPrice(big.NewInt(0)).
+		SetGasLimit(grant.GasLimit()).
+		SetAction(&grant).
+		Build()
+	se, err := action.Sign(e, identityset.PrivateKey(0))
+	require.NoError(t, err)
+	ctx = protocol.WithRunActionsCtx(
+		context.Background(),
+		protocol.RunActionsCtx{
+			Producer:    identityset.Address(0),
+			Caller:      identityset.Address(0),
+			BlockHeight: genesis.Default.NumDelegates * genesis.Default.NumSubEpochs,
+			GasPrice:    big.NewInt(0),
+		},
+	)
+	receipt, err := p.Handle(ctx, se.Action(), ws)
+	require.NoError(t, err)
+	assert.Equal(t, action.SuccessReceiptStatus, receipt.Status)
+	// Grant the block reward again should fail
+	receipt, err = p.Handle(ctx, se.Action(), ws)
+	require.NoError(t, err)
+	assert.Equal(t, action.FailureReceiptStatus, receipt.Status)
+}