Skip to content
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.

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

[iip-15] Sharing gas-fee for DApps #3844

Merged
merged 8 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions action/protocol/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ type (
CorrectGasRefund bool
SkipSystemActionNonce bool
ValidateSystemAction bool
SharedGasWithDapp bool
}

// FeatureWithHeightCtx provides feature check functions.
Expand Down Expand Up @@ -246,6 +247,7 @@ func WithFeatureCtx(ctx context.Context) context.Context {
CorrectGasRefund: g.IsOkhotsk(height),
SkipSystemActionNonce: g.IsPalau(height),
ValidateSystemAction: g.IsToBeEnabled(height),
SharedGasWithDapp: g.IsToBeEnabled(height),
},
)
}
Expand Down
68 changes: 59 additions & 9 deletions action/protocol/execution/evm/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ var (
ErrInconsistentNonce = errors.New("Nonce is not identical to executor nonce")
)

type (
// GetBlockHash gets block hash by height
GetBlockHash func(uint64) (hash.Hash256, error)

// DepositGasWithSGD deposits gas with Sharing of Gas-fee with DApps
DepositGasWithSGD func(context.Context, protocol.StateManager, address.Address, *big.Int, *big.Int) (*action.TransactionLog, error)

// SGDRegistry is the interface for handling Sharing of Gas-fee with DApps
SGDRegistry interface {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sGDRegistry?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIP-15 doc name it SGDRegistry, to the evm module, it is a registry to query the SGD status for a contract
inside chainservice, the var is named sgdIndexer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why should it be public here?

Copy link
Member Author

@dustinxie dustinxie May 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means an interface/func that EVM relies on (but will be provided outside of the EVM module), similar to GetBlockHash and DepositGasWithSGD being public

CheckContract(context.Context, string) (address.Address, uint64, bool, error)
}
)

// CanTransfer checks whether the from account has enough balance
func CanTransfer(db vm.StateDB, fromHash common.Address, balance *big.Int) bool {
return db.GetBalance(fromHash).Cmp(balance) >= 0
Expand Down Expand Up @@ -191,7 +204,8 @@ func ExecuteContract(
sm protocol.StateManager,
execution *action.Execution,
getBlockHash GetBlockHash,
depositGasFunc DepositGas,
depositGasFunc DepositGasWithSGD,
sgd SGDRegistry,
) ([]byte, *action.Receipt, error) {
ctx, span := tracer.NewSpan(ctx, "evm.ExecuteContract")
defer span.End()
Expand Down Expand Up @@ -219,7 +233,10 @@ func ExecuteContract(
}

receipt.Status = uint64(statusCode)
var burnLog *action.TransactionLog
var (
depositLog, burnLog *action.TransactionLog
consumedGas = depositGas - remainingGas
)
if featureCtx.FixDoubleChargeGas {
// Refund all deposit and, actual gas fee will be subtracted when depositing gas fee to the rewarding protocol
stateDB.AddBalance(ps.txCtx.Origin, big.NewInt(0).Mul(big.NewInt(0).SetUint64(depositGas), ps.txCtx.GasPrice))
Expand All @@ -228,19 +245,33 @@ func ExecuteContract(
remainingValue := new(big.Int).Mul(new(big.Int).SetUint64(remainingGas), ps.txCtx.GasPrice)
stateDB.AddBalance(ps.txCtx.Origin, remainingValue)
}
if depositGas-remainingGas > 0 {
if consumedGas > 0 {
burnLog = &action.TransactionLog{
Type: iotextypes.TransactionLogType_GAS_FEE,
Sender: actionCtx.Caller.String(),
Recipient: "", // burned
Amount: new(big.Int).Mul(new(big.Int).SetUint64(depositGas-remainingGas), ps.txCtx.GasPrice),
Amount: new(big.Int).Mul(new(big.Int).SetUint64(consumedGas), ps.txCtx.GasPrice),
}
}
}
var depositLog *action.TransactionLog
if depositGas-remainingGas > 0 {
gasValue := new(big.Int).Mul(new(big.Int).SetUint64(depositGas-remainingGas), ps.txCtx.GasPrice)
depositLog, err = depositGasFunc(ctx, sm, gasValue)
if consumedGas > 0 {
var (
receiver address.Address
sharedGas uint64
sharedGasFee, totalGasFee *big.Int
)
if featureCtx.SharedGasWithDapp && sgd != nil {
receiver, sharedGas, err = processSGD(ctx, sm, execution, consumedGas, sgd)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be done inside depositGasFunc?

Copy link
Member Author

@dustinxie dustinxie May 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

depositGasFunc is defined to return (*action.TransactionLog, error), need to keep the consistency
so add this processSGD as an intermediate func, to obtain receiver, sharedGas and pass that as input to depositGasFunc

if err != nil {
return nil, nil, errors.Wrap(err, "failed to process Sharing of Gas-fee with DApps")
}
}
if sharedGas > 0 {
sharedGasFee = big.NewInt(int64(sharedGas))
sharedGasFee.Mul(sharedGasFee, ps.txCtx.GasPrice)
}
totalGasFee = new(big.Int).Mul(new(big.Int).SetUint64(consumedGas), ps.txCtx.GasPrice)
depositLog, err = depositGasFunc(ctx, sm, receiver, totalGasFee, sharedGasFee)
if err != nil {
return nil, nil, err
}
Expand All @@ -267,6 +298,24 @@ func ExecuteContract(
return retval, receipt, nil
}

func processSGD(ctx context.Context, sm protocol.StateManager, execution *action.Execution, consumedGas uint64, sgd SGDRegistry,
) (address.Address, uint64, error) {
if execution.Contract() == action.EmptyAddress {
return nil, 0, nil
}

receiver, percentage, ok, err := sgd.CheckContract(ctx, execution.Contract())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls provide the benchmark result for the change. Reading the contract directly for each tx in the block might cause performance regression. IMO, an indexer built for events emitted from the contract would be a better option

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, the actual implementation is PR3845, we can review and decide the implementation there
for this PR, it only does the framework change to define and call the API

if err != nil || !ok {
return nil, 0, err
}

sharedGas := consumedGas * percentage / 100
if sharedGas > consumedGas {
sharedGas = consumedGas
}
return receiver, sharedGas, nil
}

// ReadContractStorage reads contract's storage
func ReadContractStorage(
ctx context.Context,
Expand Down Expand Up @@ -569,8 +618,9 @@ func SimulateExecution(
sm,
ex,
getBlockHash,
func(context.Context, protocol.StateManager, *big.Int) (*action.TransactionLog, error) {
func(context.Context, protocol.StateManager, address.Address, *big.Int, *big.Int) (*action.TransactionLog, error) {
return nil, nil
},
nil,
)
}
5 changes: 3 additions & 2 deletions action/protocol/execution/evm/evm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/iotexproject/go-pkgs/hash"
"github.com/iotexproject/iotex-address/address"
"github.com/iotexproject/iotex-proto/golang/iotextypes"

"github.com/iotexproject/iotex-core/action"
Expand Down Expand Up @@ -63,9 +64,9 @@ func TestExecuteContractFailure(t *testing.T) {
func(uint64) (hash.Hash256, error) {
return hash.ZeroHash256, nil
},
func(context.Context, protocol.StateManager, *big.Int) (*action.TransactionLog, error) {
func(context.Context, protocol.StateManager, address.Address, *big.Int, *big.Int) (*action.TransactionLog, error) {
return nil, nil
})
}, nil)
require.Nil(t, retval)
require.Nil(t, receipt)
require.Error(t, err)
Expand Down
7 changes: 0 additions & 7 deletions action/protocol/execution/evm/evmstatedbadapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package evm

import (
"bytes"
"context"
"encoding/hex"
"fmt"
"math/big"
Expand Down Expand Up @@ -40,12 +39,6 @@ type (
// preimageMap records the preimage of hash reported by VM
preimageMap map[common.Hash]protocol.SerializableBytes

// GetBlockHash gets block hash by height
GetBlockHash func(uint64) (hash.Hash256, error)

// DepositGas deposits gas
DepositGas func(context.Context, protocol.StateManager, *big.Int) (*action.TransactionLog, error)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these 2 are not used in here, move to evm.go where they are used

// StateDBAdapter represents the state db adapter for evm to access iotx blockchain
StateDBAdapter struct {
sm protocol.StateManager
Expand Down
9 changes: 5 additions & 4 deletions action/protocol/execution/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,19 @@ const (
// Protocol defines the protocol of handling executions
type Protocol struct {
getBlockHash evm.GetBlockHash
depositGas evm.DepositGas
depositGas evm.DepositGasWithSGD
addr address.Address
sgdRegistry evm.SGDRegistry
}

// NewProtocol instantiates the protocol of exeuction
func NewProtocol(getBlockHash evm.GetBlockHash, depostGas evm.DepositGas) *Protocol {
func NewProtocol(getBlockHash evm.GetBlockHash, depositGasWithSGD evm.DepositGasWithSGD, sgd evm.SGDRegistry) *Protocol {
h := hash.Hash160b([]byte(_protocolID))
addr, err := address.FromBytes(h[:])
if err != nil {
log.L().Panic("Error when constructing the address of vote protocol", zap.Error(err))
}
return &Protocol{getBlockHash: getBlockHash, depositGas: depostGas, addr: addr}
return &Protocol{getBlockHash: getBlockHash, depositGas: depositGasWithSGD, addr: addr, sgdRegistry: sgd}
}

// FindProtocol finds the registered protocol from registry
Expand All @@ -65,7 +66,7 @@ func (p *Protocol) Handle(ctx context.Context, act action.Action, sm protocol.St
if !ok {
return nil, nil
}
_, receipt, err := evm.ExecuteContract(ctx, sm, exec, p.getBlockHash, p.depositGas)
_, receipt, err := evm.ExecuteContract(ctx, sm, exec, p.getBlockHash, p.depositGas, p.sgdRegistry)

if err != nil {
return nil, errors.Wrap(err, "failed to execute contract")
Expand Down
6 changes: 3 additions & 3 deletions action/protocol/execution/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func (sct *SmartContractTest) prepareBlockchain(
r.NoError(reward.Register(registry))

r.NotNil(bc)
execution := NewProtocol(dao.GetBlockHash, rewarding.DepositGas)
execution := NewProtocol(dao.GetBlockHash, rewarding.DepositGasWithSGD, nil)
r.NoError(execution.Register(registry))
r.NoError(bc.Start(ctx))

Expand Down Expand Up @@ -605,7 +605,7 @@ func TestProtocol_Validate(t *testing.T) {
require := require.New(t)
p := NewProtocol(func(uint64) (hash.Hash256, error) {
return hash.ZeroHash256, nil
}, rewarding.DepositGas)
}, rewarding.DepositGasWithSGD, nil)

ex, err := action.NewExecution("2", uint64(1), big.NewInt(0), uint64(0), big.NewInt(0), make([]byte, 32684))
require.NoError(err)
Expand Down Expand Up @@ -675,7 +675,7 @@ func TestProtocol_Handle(t *testing.T) {
protocol.NewGenericValidator(sf, accountutil.AccountState),
)),
)
exeProtocol := NewProtocol(dao.GetBlockHash, rewarding.DepositGas)
exeProtocol := NewProtocol(dao.GetBlockHash, rewarding.DepositGasWithSGD, nil)
require.NoError(exeProtocol.Register(registry))
require.NoError(bc.Start(ctx))
require.NotNil(bc)
Expand Down
3 changes: 2 additions & 1 deletion action/protocol/poll/consortium.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,10 @@ func (cc *consortiumCommittee) CreateGenesisStates(ctx context.Context, sm proto
func(height uint64) (hash.Hash256, error) {
return hash.ZeroHash256, nil
},
func(ctx context.Context, sm protocol.StateManager, amount *big.Int) (*action.TransactionLog, error) {
func(context.Context, protocol.StateManager, address.Address, *big.Int, *big.Int) (*action.TransactionLog, error) {
return nil, nil
},
nil,
)
if err != nil {
return err
Expand Down
3 changes: 2 additions & 1 deletion action/protocol/poll/staking_committee.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,10 @@ func (sc *stakingCommittee) CreateGenesisStates(ctx context.Context, sm protocol
func(height uint64) (hash.Hash256, error) {
return hash.ZeroHash256, nil
},
func(ctx context.Context, sm protocol.StateManager, amount *big.Int) (*action.TransactionLog, error) {
func(context.Context, protocol.StateManager, address.Address, *big.Int, *big.Int) (*action.TransactionLog, error) {
return nil, nil
},
nil,
)
if err != nil {
return err
Expand Down
57 changes: 55 additions & 2 deletions action/protocol/rewarding/fund.go
envestcc marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ func (p *Protocol) Deposit(
sm protocol.StateManager,
amount *big.Int,
transactionLogType iotextypes.TransactionLogType,
) (*action.TransactionLog, error) {
// fallback to regular case by setting sgdAmount = nil
return p.deposit(ctx, sm, nil, amount, nil, transactionLogType)
}

func (p *Protocol) deposit(
ctx context.Context,
sm protocol.StateManager,
receiver address.Address,
amount, sgdAmount *big.Int,
transactionLogType iotextypes.TransactionLogType,
) (*action.TransactionLog, error) {
actionCtx := protocol.MustGetActionCtx(ctx)
accountCreationOpts := []state.AccountCreationOption{}
Expand All @@ -85,8 +96,18 @@ func (p *Protocol) Deposit(
if _, err := p.state(ctx, sm, _fundKey, &f); err != nil {
return nil, err
}
f.totalBalance = big.NewInt(0).Add(f.totalBalance, amount)
f.unclaimedBalance = big.NewInt(0).Add(f.unclaimedBalance, amount)
f.totalBalance.Add(f.totalBalance, amount)
f.unclaimedBalance.Add(f.unclaimedBalance, amount)
if !isZero(sgdAmount) {
envestcc marked this conversation as resolved.
Show resolved Hide resolved
f.unclaimedBalance.Sub(f.unclaimedBalance, sgdAmount)
if f.unclaimedBalance.Sign() == -1 {
return nil, errors.New("no enough available balance")
}
// grant sgd amount to receiver
if err := p.grantToAccount(ctx, sm, receiver, sgdAmount); err != nil {
envestcc marked this conversation as resolved.
Show resolved Hide resolved
return nil, err
}
}
if err := p.putState(ctx, sm, _fundKey, &f); err != nil {
return nil, err
}
Expand Down Expand Up @@ -146,3 +167,35 @@ func DepositGas(ctx context.Context, sm protocol.StateManager, amount *big.Int)
}
return rp.Deposit(ctx, sm, amount, iotextypes.TransactionLogType_GAS_FEE)
Liuhaai marked this conversation as resolved.
Show resolved Hide resolved
}

// DepositGasWithSGD deposits gas into the rewarding fund with Sharing of Gas-fee with DApps
func DepositGasWithSGD(ctx context.Context, sm protocol.StateManager, sgdReceiver address.Address, totalAmount, sgdAmount *big.Int,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when will DepositGas() be used, and when will DepositGasWithSGD() be used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DepositGas() is not touched, and will be used by other protocols.
DepositGasWithSGD() will be used by evm/execution protocol

) (*action.TransactionLog, error) {
if isZero(sgdAmount) {
Liuhaai marked this conversation as resolved.
Show resolved Hide resolved
// fallback to regular case if SGD amount is zero
return DepositGas(ctx, sm, totalAmount)
}
if sgdReceiver == nil {
// a valid SGD amount but no valid receiver address
return nil, errors.New("no valid receiver address to receive the Sharing of Gas-fee with DApps")
}
// TODO: we bypass the gas deposit for the actions in genesis block. Later we should remove this after we remove
// genesis actions
blkCtx := protocol.MustGetBlockCtx(ctx)
if blkCtx.BlockHeight == 0 {
return nil, nil
}
reg, ok := protocol.GetRegistry(ctx)
if !ok {
return nil, nil
}
rp := FindProtocol(reg)
if rp == nil {
return nil, nil
}
return rp.deposit(ctx, sm, sgdReceiver, totalAmount, sgdAmount, iotextypes.TransactionLogType_GAS_FEE)
}

func isZero(a *big.Int) bool {
return a == nil || len(a.Bytes()) == 0
}
7 changes: 7 additions & 0 deletions action/protocol/rewarding/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,14 @@ func TestStateCheckLegacy(t *testing.T) {
require.Equal(tests[useV2].before.Add(tests[useV2].before, tests[useV2].add), acc.balance)
}
if i == 0 {
// grant to existing addr
require.NoError(p.grantToAccount(ctx, sm, addr, tests[useV2].add))
// grant to new addr
newAddr := identityset.Address(4 + useV2)
require.NoError(p.grantToAccount(ctx, sm, newAddr, tests[useV2].add))
_, err = p.state(ctx, sm, append(_adminKey, newAddr.Bytes()...), &acc)
require.NoError(err)
require.Equal(acc.balance, tests[useV2].add)
}
}

Expand Down
2 changes: 1 addition & 1 deletion api/serverV2_integrity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func setupChain(cfg testConfig) (blockchain.Blockchain, blockdao.BlockDAO, block
}()

acc := account.NewProtocol(rewarding.DepositGas)
evm := execution.NewProtocol(dao.GetBlockHash, rewarding.DepositGas)
evm := execution.NewProtocol(dao.GetBlockHash, rewarding.DepositGasWithSGD, nil)
p := poll.NewLifeLongDelegatesProtocol(cfg.genesis.Delegates)
rolldposProtocol := rolldpos.NewProtocol(
genesis.Default.NumCandidateDelegates,
Expand Down
2 changes: 1 addition & 1 deletion blockchain/integrity/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func newChainInDB() (blockchain.Blockchain, actpool.ActPool, error) {
if bc == nil {
return nil, nil, errors.New("pointer is nil")
}
ep := execution.NewProtocol(dao.GetBlockHash, rewarding.DepositGas)
ep := execution.NewProtocol(dao.GetBlockHash, rewarding.DepositGasWithSGD, nil)
if err = ep.Register(registry); err != nil {
return nil, nil, err
}
Expand Down
Loading