Skip to content

Commit

Permalink
Handle empty reward address and fix available balance acccounting (#826)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zjshen14
zjshen14 committed Mar 24, 2019
1 parent c02d677 commit ca62aa2
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 24 deletions.
2 changes: 1 addition & 1 deletion action/protocol/rewarding/admin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (p *Protocol) NumDelegatesForFoundationBonus(_ context.Context, sm protocol
if err := p.state(sm, adminKey, &a); err != nil {
return 0, err
}
return a.numDelegatesForEpochReward, nil
return a.numDelegatesForFoundationBonus, nil
}

// ProductivityThreshold returns the productivity threshold
Expand Down
24 changes: 16 additions & 8 deletions action/protocol/rewarding/protocol_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,21 @@ import (
"math/big"
"testing"

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

"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"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/iotexproject/iotex-core/action"
"github.com/iotexproject/iotex-core/action/protocol"
accountutil "github.com/iotexproject/iotex-core/action/protocol/account/util"
"github.com/iotexproject/iotex-core/action/protocol/rolldpos"
"github.com/iotexproject/iotex-core/address"
"github.com/iotexproject/iotex-core/blockchain/genesis"
"github.com/iotexproject/iotex-core/config"
"github.com/iotexproject/iotex-core/pkg/unit"
"github.com/iotexproject/iotex-core/state"
"github.com/iotexproject/iotex-core/state/factory"
"github.com/iotexproject/iotex-core/test/identityset"
"github.com/iotexproject/iotex-core/test/mock/mock_chainmanager"
"github.com/iotexproject/iotex-core/test/testaddress"
)
Expand Down Expand Up @@ -154,6 +150,18 @@ func testProtocol(t *testing.T, test func(*testing.T, context.Context, factory.F
epochReward, err := p.EpochReward(ctx, ws)
require.NoError(t, err)
assert.Equal(t, big.NewInt(100), epochReward)
fb, err := p.FoundationBonus(ctx, ws)
require.NoError(t, err)
assert.Equal(t, big.NewInt(5), fb)
ndffb, err := p.NumDelegatesForFoundationBonus(ctx, ws)
require.NoError(t, err)
assert.Equal(t, uint64(5), ndffb)
fble, err := p.FoundationBonusLastEpoch(ctx, ws)
require.NoError(t, err)
assert.Equal(t, uint64(365), fble)
pt, err := p.ProductivityThreshold(ctx, ws)
require.NoError(t, err)
assert.Equal(t, uint64(50), pt)

totalBalance, err := p.TotalBalance(ctx, ws)
require.NoError(t, err)
Expand Down
53 changes: 39 additions & 14 deletions action/protocol/rewarding/reward.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/iotexproject/iotex-core/action/protocol/rewarding/rewardingpb"
"github.com/iotexproject/iotex-core/address"
"github.com/iotexproject/iotex-core/pkg/enc"
"github.com/iotexproject/iotex-core/pkg/log"
"github.com/iotexproject/iotex-core/state"
)

Expand Down Expand Up @@ -69,13 +70,7 @@ func (p *Protocol) GrantBlockReward(
if err := p.assertNoRewardYet(sm, blockRewardHistoryKeyPrefix, raCtx.BlockHeight); err != nil {
return err
}
a := admin{}
if err := p.state(sm, adminKey, &a); err != nil {
return err
}
if err := p.updateAvailableBalance(sm, a.blockReward); err != nil {
return err
}

// Get the reward address for the block producer
epochNum := p.rp.GetEpochNum(raCtx.BlockHeight)
candidates, err := p.cm.CandidatesByHeight(p.rp.GetEpochHeight(epochNum))
Expand All @@ -90,10 +85,20 @@ func (p *Protocol) GrantBlockReward(
break
}
}
// If reward address doesn't exist, do nothing
if rewardAddrStr == "" {
return errors.Errorf("Producer %s doesn't have a reward address", producerAddrStr)
log.S().Warnf("Producer %s doesn't have a reward address", producerAddrStr)
return nil
}
rewardAddr, err := address.FromString(rewardAddrStr)

a := admin{}
if err := p.state(sm, adminKey, &a); err != nil {
return err
}
if err := p.updateAvailableBalance(sm, a.blockReward); err != nil {
return err
}
if err != nil {
return err
}
Expand All @@ -120,9 +125,6 @@ func (p *Protocol) GrantEpochReward(
if err := p.state(sm, adminKey, &a); err != nil {
return err
}
if err := p.updateAvailableBalance(sm, a.epochReward); err != nil {
return err
}
// We need to consistently use the votes on of first block height in this epoch
candidates, err := p.cm.CandidatesByHeight(p.rp.GetEpochHeight(epochNum))
if err != nil {
Expand All @@ -139,10 +141,16 @@ func (p *Protocol) GrantEpochReward(
if err != nil {
return err
}
actualTotalReward := big.NewInt(0)
for i := range addrs {
// If reward address doesn't exist, do nothing
if addrs[i] == nil {
continue
}
if err := p.grantToAccount(sm, addrs[i], amounts[i]); err != nil {
return err
}
actualTotalReward = big.NewInt(0).Add(actualTotalReward, amounts[i])
}

// Reward additional bootstrap bonus
Expand All @@ -152,15 +160,26 @@ func (p *Protocol) GrantEpochReward(
l = a.numDelegatesForFoundationBonus
}
for i := uint64(0); i < l; i++ {
// If reward address doesn't exist, do nothing
if candidates[i].RewardAddress == "" {
log.S().Warnf("Candidate %s doesn't have a reward address", candidates[i].Address)
continue
}
rewardAddr, err := address.FromString(candidates[i].RewardAddress)
if err != nil {
return err
}
if err := p.grantToAccount(sm, rewardAddr, a.foundationBonus); err != nil {
return err
}
actualTotalReward = big.NewInt(0).Add(actualTotalReward, a.foundationBonus)
}
}

// Update actual reward
if err := p.updateAvailableBalance(sm, actualTotalReward); err != nil {
return err
}
return p.updateRewardHistory(sm, epochRewardHistoryKeyPrefix, epochNum)
}

Expand Down Expand Up @@ -310,9 +329,15 @@ func (p *Protocol) splitEpochReward(
totalWeight := big.NewInt(0)
rewardAddrs := make([]address.Address, 0)
for _, candidate := range candidates {
rewardAddr, err := address.FromString(candidate.RewardAddress)
if err != nil {
return nil, nil, err
var rewardAddr address.Address
var err error
if candidate.RewardAddress != "" {
rewardAddr, err = address.FromString(candidate.RewardAddress)
if err != nil {
return nil, nil, err
}
} else {
log.S().Warnf("Candidate %s doesn't have a reward address", candidate.Address)
}
rewardAddrs = append(rewardAddrs, rewardAddr)
totalWeight = big.NewInt(0).Add(totalWeight, candidate.Votes)
Expand Down
129 changes: 128 additions & 1 deletion action/protocol/rewarding/reward_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,21 @@ import (
"math/big"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/iotexproject/iotex-core/action/protocol"
accountutil "github.com/iotexproject/iotex-core/action/protocol/account/util"
"github.com/iotexproject/iotex-core/action/protocol/rolldpos"
"github.com/iotexproject/iotex-core/blockchain/genesis"
"github.com/iotexproject/iotex-core/config"
"github.com/iotexproject/iotex-core/pkg/hash"
"github.com/iotexproject/iotex-core/pkg/unit"
"github.com/iotexproject/iotex-core/state"
"github.com/iotexproject/iotex-core/state/factory"
"github.com/iotexproject/iotex-core/test/identityset"
"github.com/iotexproject/iotex-core/test/mock/mock_chainmanager"
"github.com/iotexproject/iotex-core/test/testaddress"
)

Expand Down Expand Up @@ -84,7 +91,7 @@ func TestProtocol_GrantEpochReward(t *testing.T) {
require.NoError(t, err)
availableBalance, err := p.AvailableBalance(ctx, ws)
require.NoError(t, err)
assert.Equal(t, big.NewInt(100), availableBalance)
assert.Equal(t, big.NewInt(95), availableBalance)
// Operator shouldn't get reward
unclaimedBalance, err := p.UnclaimedBalance(ctx, ws, testaddress.Addrinfo["producer"])
require.NoError(t, err)
Expand Down Expand Up @@ -233,3 +240,123 @@ func TestProtocol_ClaimReward(t *testing.T) {
require.Error(t, p.Claim(claimCtx, ws, big.NewInt(1)))
}, false)
}

func TestProtocol_NoRewardAddr(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)
chain.EXPECT().CandidatesByHeight(gomock.Any()).Return([]*state.Candidate{
{
Address: identityset.Address(0).String(),
Votes: unit.ConvertIotxToRau(1000000),
RewardAddress: "",
},
{
Address: identityset.Address(1).String(),
Votes: unit.ConvertIotxToRau(1000000),
RewardAddress: identityset.Address(1).String(),
},
}, nil).AnyTimes()
chain.EXPECT().ProductivityByEpoch(gomock.Any()).Return(
uint64(19),
map[string]uint64{
identityset.Address(0).String(): 9,
identityset.Address(1).String(): 10,
},
nil,
).AnyTimes()
p := NewProtocol(chain, rolldpos.NewProtocol(
genesis.Default.NumCandidateDelegates,
genesis.Default.NumDelegates,
genesis.Default.NumSubEpochs,
))

// Initialize the protocol
ctx := protocol.WithRunActionsCtx(
context.Background(),
protocol.RunActionsCtx{
BlockHeight: 0,
},
)
ws, err := stateDB.NewWorkingSet()
require.NoError(t, err)
require.NoError(
t,
p.Initialize(
ctx,
ws,
big.NewInt(0),
big.NewInt(10),
big.NewInt(100),
10,
nil,
big.NewInt(5),
5,
365,
50,
))
require.NoError(t, stateDB.Commit(ws))

// Create a test account with 1000 token
ws, err = stateDB.NewWorkingSet()
require.NoError(t, err)
_, err = accountutil.LoadOrCreateAccount(ws, identityset.Address(0).String(), big.NewInt(1000))
require.NoError(t, err)
require.NoError(t, stateDB.Commit(ws))

ctx = protocol.WithRunActionsCtx(
context.Background(),
protocol.RunActionsCtx{
Producer: identityset.Address(0),
Caller: identityset.Address(0),
BlockHeight: genesis.Default.NumDelegates * genesis.Default.NumSubEpochs,
},
)
ws, err = stateDB.NewWorkingSet()
require.NoError(t, err)
require.NoError(t, p.Deposit(ctx, ws, big.NewInt(200)))
require.NoError(t, stateDB.Commit(ws))

// Grant block reward
ws, err = stateDB.NewWorkingSet()
require.NoError(t, err)
require.NoError(t, p.GrantBlockReward(ctx, ws))
require.NoError(t, stateDB.Commit(ws))

ws, err = stateDB.NewWorkingSet()
require.NoError(t, err)
availableBalance, err := p.AvailableBalance(ctx, ws)
require.NoError(t, err)
assert.Equal(t, big.NewInt(200), availableBalance)
unclaimedBalance, err := p.UnclaimedBalance(ctx, ws, identityset.Address(0))
require.NoError(t, err)
assert.Equal(t, big.NewInt(0), unclaimedBalance)

// Grant epoch reward
ws, err = stateDB.NewWorkingSet()
require.NoError(t, err)
require.NoError(t, p.GrantEpochReward(ctx, ws))
require.NoError(t, stateDB.Commit(ws))

ws, err = stateDB.NewWorkingSet()
require.NoError(t, err)
availableBalance, err = p.AvailableBalance(ctx, ws)
require.NoError(t, err)
assert.Equal(t, big.NewInt(145), availableBalance)
unclaimedBalance, err = p.UnclaimedBalance(ctx, ws, identityset.Address(0))
require.NoError(t, err)
assert.Equal(t, big.NewInt(0), unclaimedBalance)
// It doesn't affect others to get reward
unclaimedBalance, err = p.UnclaimedBalance(ctx, ws, identityset.Address(1))
require.NoError(t, err)
assert.Equal(t, big.NewInt(55), unclaimedBalance)
}

0 comments on commit ca62aa2

Please sign in to comment.