Skip to content

Commit 5576ef9

Browse files
zjshen14zjshen14
authored
Fix the bugs of double charging gas fee (iotexproject#1141)
1 parent e8b4ab9 commit 5576ef9

23 files changed

+173
-105
lines changed

‎action/protocol/account/protocol.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,22 @@ import (
2626
const ProtocolID = "account"
2727

2828
// Protocol defines the protocol of handling account
29-
type Protocol struct{ addr address.Address }
29+
type Protocol struct {
30+
addr address.Address
31+
pacificHeight uint64
32+
}
3033

3134
// NewProtocol instantiates the protocol of account
32-
func NewProtocol() *Protocol {
35+
func NewProtocol(pacificHeight uint64) *Protocol {
3336
h := hash.Hash160b([]byte(ProtocolID))
3437
addr, err := address.FromBytes(h[:])
3538
if err != nil {
3639
log.L().Panic("Error when constructing the address of account protocol", zap.Error(err))
3740
}
38-
return &Protocol{addr: addr}
41+
return &Protocol{
42+
addr: addr,
43+
pacificHeight: pacificHeight,
44+
}
3945
}
4046

4147
// Handle handles an account

‎action/protocol/account/protocol_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestProtocol_Initialize(t *testing.T) {
7171
require.NoError(t, stateDB.Stop(context.Background()))
7272
}()
7373

74-
p := NewProtocol()
74+
p := NewProtocol(0)
7575

7676
ws, err := stateDB.NewWorkingSet()
7777
require.NoError(t, err)

‎action/protocol/account/transfer.go

+22-15
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ import (
1010
"context"
1111
"math/big"
1212

13-
"github.com/pkg/errors"
14-
1513
"github.com/iotexproject/go-pkgs/hash"
1614
"github.com/iotexproject/iotex-address/address"
15+
"github.com/pkg/errors"
16+
1717
"github.com/iotexproject/iotex-core/action"
1818
"github.com/iotexproject/iotex-core/action/protocol"
1919
accountutil "github.com/iotexproject/iotex-core/action/protocol/account/util"
@@ -52,13 +52,16 @@ func (p *Protocol) handleTransfer(ctx context.Context, act action.Action, sm pro
5252
)
5353
}
5454

55-
// charge sender gas
56-
if err := sender.SubBalance(gasFee); err != nil {
57-
return nil, errors.Wrapf(err, "failed to charge the gas for sender %s", raCtx.Caller.String())
58-
}
59-
if err := rewarding.DepositGas(ctx, sm, gasFee, raCtx.Registry); err != nil {
60-
return nil, err
55+
if raCtx.BlockHeight < p.pacificHeight {
56+
// charge sender gas
57+
if err := sender.SubBalance(gasFee); err != nil {
58+
return nil, errors.Wrapf(err, "failed to charge the gas for sender %s", raCtx.Caller.String())
59+
}
60+
if err := rewarding.DepositGas(ctx, sm, gasFee, raCtx.Registry); err != nil {
61+
return nil, err
62+
}
6163
}
64+
6265
recipientAddr, err := address.FromString(tsf.Recipient())
6366
if err != nil {
6467
return nil, errors.Wrapf(err, "failed to decode recipient address %s", tsf.Recipient())
@@ -71,6 +74,11 @@ func (p *Protocol) handleTransfer(ctx context.Context, act action.Action, sm pro
7174
if err := accountutil.StoreAccount(sm, raCtx.Caller.String(), sender); err != nil {
7275
return nil, errors.Wrap(err, "failed to update pending account changes to trie")
7376
}
77+
if raCtx.BlockHeight >= p.pacificHeight {
78+
if err := rewarding.DepositGas(ctx, sm, gasFee, raCtx.Registry); err != nil {
79+
return nil, err
80+
}
81+
}
7482
return &action.Receipt{
7583
Status: action.FailureReceiptStatus,
7684
BlockHeight: raCtx.BlockHeight,
@@ -80,13 +88,6 @@ func (p *Protocol) handleTransfer(ctx context.Context, act action.Action, sm pro
8088
}, nil
8189
}
8290

83-
if tsf.Amount().Cmp(sender.Balance) == 1 {
84-
return nil, errors.Wrapf(
85-
state.ErrNotEnoughBalance,
86-
"failed to verify the Balance of sender %s",
87-
raCtx.Caller.String(),
88-
)
89-
}
9091
// update sender Balance
9192
if err := sender.SubBalance(tsf.Amount()); err != nil {
9293
return nil, errors.Wrapf(err, "failed to update the Balance of sender %s", raCtx.Caller.String())
@@ -110,6 +111,12 @@ func (p *Protocol) handleTransfer(ctx context.Context, act action.Action, sm pro
110111
if err := accountutil.StoreAccount(sm, tsf.Recipient(), recipient); err != nil {
111112
return nil, errors.Wrap(err, "failed to update pending account changes to trie")
112113
}
114+
115+
if raCtx.BlockHeight >= p.pacificHeight {
116+
if err := rewarding.DepositGas(ctx, sm, gasFee, raCtx.Registry); err != nil {
117+
return nil, err
118+
}
119+
}
113120
return &action.Receipt{
114121
Status: action.SuccessReceiptStatus,
115122
BlockHeight: raCtx.BlockHeight,

‎action/protocol/account/transfer_test.go

+39-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ import (
1212
"strings"
1313
"testing"
1414

15+
"github.com/golang/mock/gomock"
16+
"github.com/iotexproject/iotex-core/action/protocol/rolldpos"
17+
"github.com/iotexproject/iotex-core/test/mock/mock_chainmanager"
18+
19+
"github.com/iotexproject/iotex-core/action/protocol/rewarding"
20+
1521
"github.com/iotexproject/iotex-core/testutil"
1622

1723
"github.com/pkg/errors"
@@ -40,7 +46,36 @@ func TestProtocol_HandleTransfer(t *testing.T) {
4046
ws, err := sf.NewWorkingSet()
4147
require.NoError(err)
4248

43-
p := NewProtocol()
49+
p := NewProtocol(0)
50+
51+
ctrl := gomock.NewController(t)
52+
defer ctrl.Finish()
53+
cm := mock_chainmanager.NewMockChainManager(ctrl)
54+
reward := rewarding.NewProtocol(cm, rolldpos.NewProtocol(1, 1, 1))
55+
registry := protocol.Registry{}
56+
require.NoError(registry.Register(rewarding.ProtocolID, reward))
57+
require.NoError(
58+
reward.Initialize(
59+
protocol.WithRunActionsCtx(context.Background(),
60+
protocol.RunActionsCtx{
61+
BlockHeight: 0,
62+
Producer: testaddress.Addrinfo["producer"],
63+
Caller: testaddress.Addrinfo["alfa"],
64+
GasLimit: testutil.TestGasLimit,
65+
Registry: &registry,
66+
}),
67+
ws,
68+
big.NewInt(0),
69+
big.NewInt(0),
70+
big.NewInt(0),
71+
1,
72+
nil,
73+
big.NewInt(0),
74+
0,
75+
0,
76+
0,
77+
),
78+
)
4479

4580
accountAlfa := state.Account{
4681
Balance: big.NewInt(50005),
@@ -68,10 +103,12 @@ func TestProtocol_HandleTransfer(t *testing.T) {
68103
require.NoError(err)
69104
ctx = protocol.WithRunActionsCtx(context.Background(),
70105
protocol.RunActionsCtx{
106+
BlockHeight: 1,
71107
Producer: testaddress.Addrinfo["producer"],
72108
Caller: testaddress.Addrinfo["alfa"],
73109
GasLimit: testutil.TestGasLimit,
74110
IntrinsicGas: gas,
111+
Registry: &registry,
75112
})
76113
receipt, err := p.Handle(ctx, transfer, ws)
77114
require.NoError(err)
@@ -111,7 +148,7 @@ func TestProtocol_HandleTransfer(t *testing.T) {
111148

112149
func TestProtocol_ValidateTransfer(t *testing.T) {
113150
require := require.New(t)
114-
protocol := NewProtocol()
151+
protocol := NewProtocol(0)
115152
// Case I: Oversized data
116153
tmpPayload := [32769]byte{}
117154
payload := tmpPayload[:]

‎action/protocol/execution/evm/evm.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ func ExecuteContract(
138138
sm protocol.StateManager,
139139
execution *action.Execution,
140140
cm protocol.ChainManager,
141+
pacificHeight uint64,
141142
) ([]byte, *action.Receipt, error) {
142143
raCtx := protocol.MustGetRunActionsCtx(ctx)
143144
stateDB := NewStateDBAdapter(cm, sm, raCtx.BlockHeight, execution.Hash())
@@ -160,9 +161,14 @@ func ExecuteContract(
160161
} else {
161162
receipt.Status = action.SuccessReceiptStatus
162163
}
163-
if remainingGas > 0 {
164-
remainingValue := new(big.Int).Mul(new(big.Int).SetUint64(remainingGas), ps.context.GasPrice)
165-
stateDB.AddBalance(ps.context.Origin, remainingValue)
164+
if raCtx.BlockHeight >= pacificHeight {
165+
// Refund all deposit and, actual gas fee will be subtracted when depositing gas fee to the rewarding protocol
166+
stateDB.AddBalance(ps.context.Origin, big.NewInt(0).Mul(big.NewInt(0).SetUint64(depositGas), ps.context.GasPrice))
167+
} else {
168+
if remainingGas > 0 {
169+
remainingValue := new(big.Int).Mul(new(big.Int).SetUint64(remainingGas), ps.context.GasPrice)
170+
stateDB.AddBalance(ps.context.Origin, remainingValue)
171+
}
166172
}
167173
if depositGas-remainingGas > 0 {
168174
gasValue := new(big.Int).Mul(new(big.Int).SetUint64(depositGas-remainingGas), ps.context.GasPrice)

‎action/protocol/execution/evm/evm_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func TestExecuteContractFailure(t *testing.T) {
8686
GasLimit: testutil.TestGasLimit,
8787
})
8888

89-
retval, receipt, err := ExecuteContract(ctx, sm, e, cm)
89+
retval, receipt, err := ExecuteContract(ctx, sm, e, cm, 0)
9090
require.Nil(t, retval)
9191
require.Nil(t, receipt)
9292
require.Error(t, err)

‎action/protocol/execution/protocol.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,19 @@ const (
3030

3131
// Protocol defines the protocol of handling executions
3232
type Protocol struct {
33-
cm protocol.ChainManager
34-
addr address.Address
33+
cm protocol.ChainManager
34+
addr address.Address
35+
pacificHeight uint64
3536
}
3637

3738
// NewProtocol instantiates the protocol of exeuction
38-
func NewProtocol(cm protocol.ChainManager) *Protocol {
39+
func NewProtocol(cm protocol.ChainManager, pacificHeight uint64) *Protocol {
3940
h := hash.Hash160b([]byte(ProtocolID))
4041
addr, err := address.FromBytes(h[:])
4142
if err != nil {
4243
log.L().Panic("Error when constructing the address of vote protocol", zap.Error(err))
4344
}
44-
return &Protocol{cm: cm, addr: addr}
45+
return &Protocol{cm: cm, addr: addr, pacificHeight: pacificHeight}
4546
}
4647

4748
// Handle handles an execution
@@ -50,7 +51,7 @@ func (p *Protocol) Handle(ctx context.Context, act action.Action, sm protocol.St
5051
if !ok {
5152
return nil, nil
5253
}
53-
_, receipt, err := evm.ExecuteContract(ctx, sm, exec, p.cm)
54+
_, receipt, err := evm.ExecuteContract(ctx, sm, exec, p.cm, p.pacificHeight)
5455

5556
if err != nil {
5657
return nil, errors.Wrap(err, "failed to execute contract")

‎action/protocol/execution/protocol_test.go

+15-11
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/iotexproject/iotex-core/action/protocol/account"
3232
accountutil "github.com/iotexproject/iotex-core/action/protocol/account/util"
3333
"github.com/iotexproject/iotex-core/action/protocol/execution/evm"
34+
"github.com/iotexproject/iotex-core/action/protocol/rewarding"
3435
"github.com/iotexproject/iotex-core/action/protocol/rolldpos"
3536
"github.com/iotexproject/iotex-core/blockchain"
3637
"github.com/iotexproject/iotex-core/blockchain/genesis"
@@ -258,22 +259,25 @@ func (sct *SmartContractTest) prepareBlockchain(
258259
cfg.Chain.EnableAsyncIndexWrite = false
259260
cfg.Genesis.EnableGravityChainVoting = false
260261
registry := protocol.Registry{}
261-
acc := account.NewProtocol()
262-
registry.Register(account.ProtocolID, acc)
262+
acc := account.NewProtocol(0)
263+
r.NoError(registry.Register(account.ProtocolID, acc))
263264
rp := rolldpos.NewProtocol(cfg.Genesis.NumCandidateDelegates, cfg.Genesis.NumDelegates, cfg.Genesis.NumSubEpochs)
264-
registry.Register(rolldpos.ProtocolID, rp)
265+
r.NoError(registry.Register(rolldpos.ProtocolID, rp))
265266
bc := blockchain.NewBlockchain(
266267
cfg,
267268
blockchain.InMemDaoOption(),
268269
blockchain.InMemStateFactoryOption(),
269270
blockchain.RegistryOption(&registry),
270271
)
272+
reward := rewarding.NewProtocol(bc, rp)
273+
r.NoError(registry.Register(rewarding.ProtocolID, reward))
274+
271275
r.NotNil(bc)
272276
bc.Validator().AddActionEnvelopeValidators(protocol.NewGenericValidator(bc, genesis.Default.ActionGasLimit))
273-
bc.Validator().AddActionValidators(account.NewProtocol(), NewProtocol(bc))
277+
bc.Validator().AddActionValidators(account.NewProtocol(0), NewProtocol(bc, 0), reward)
274278
sf := bc.GetFactory()
275279
r.NotNil(sf)
276-
sf.AddActionHandlers(NewProtocol(bc))
280+
sf.AddActionHandlers(NewProtocol(bc, 0), reward)
277281
r.NoError(bc.Start(ctx))
278282
ws, err := sf.NewWorkingSet()
279283
r.NoError(err)
@@ -408,21 +412,21 @@ func TestProtocol_Handle(t *testing.T) {
408412
cfg.Chain.EnableAsyncIndexWrite = false
409413
cfg.Genesis.EnableGravityChainVoting = false
410414
registry := protocol.Registry{}
411-
acc := account.NewProtocol()
412-
registry.Register(account.ProtocolID, acc)
415+
acc := account.NewProtocol(0)
416+
require.NoError(registry.Register(account.ProtocolID, acc))
413417
rp := rolldpos.NewProtocol(cfg.Genesis.NumCandidateDelegates, cfg.Genesis.NumDelegates, cfg.Genesis.NumSubEpochs)
414-
registry.Register(rolldpos.ProtocolID, rp)
418+
require.NoError(registry.Register(rolldpos.ProtocolID, rp))
415419
bc := blockchain.NewBlockchain(
416420
cfg,
417421
blockchain.DefaultStateFactoryOption(),
418422
blockchain.BoltDBDaoOption(),
419423
blockchain.RegistryOption(&registry),
420424
)
421425
bc.Validator().AddActionEnvelopeValidators(protocol.NewGenericValidator(bc, genesis.Default.ActionGasLimit))
422-
bc.Validator().AddActionValidators(account.NewProtocol(), NewProtocol(bc))
426+
bc.Validator().AddActionValidators(account.NewProtocol(0), NewProtocol(bc, 0))
423427
sf := bc.GetFactory()
424428
require.NotNil(sf)
425-
sf.AddActionHandlers(NewProtocol(bc))
429+
sf.AddActionHandlers(NewProtocol(bc, 0))
426430

427431
require.NoError(bc.Start(ctx))
428432
require.NotNil(bc)
@@ -691,7 +695,7 @@ func TestProtocol_Validate(t *testing.T) {
691695
defer ctrl.Finish()
692696

693697
mbc := mock_blockchain.NewMockBlockchain(ctrl)
694-
protocol := NewProtocol(mbc)
698+
protocol := NewProtocol(mbc, 0)
695699
// Case I: Oversized data
696700
tmpPayload := [32769]byte{}
697701
data := tmpPayload[:]

‎action/protocol/multichain/mainchain/startsubchain_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ func TestStartSubChainInGenesis(t *testing.T) {
345345

346346
ctx := context.Background()
347347
registry := protocol.Registry{}
348-
acc := account.NewProtocol()
348+
acc := account.NewProtocol(0)
349349
registry.Register(account.ProtocolID, acc)
350350
bc := blockchain.NewBlockchain(
351351
cfg,

0 commit comments

Comments
 (0)