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

fix SimulateExecution, add sgd contract check #3983

Merged
merged 7 commits into from
Nov 23, 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
18 changes: 6 additions & 12 deletions action/protocol/execution/evm/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@
BaseFee: new(big.Int),
}
if vmCfg, ok := protocol.GetVMConfigCtx(ctx); ok {
vmConfig = vmCfg

Check warning on line 180 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L180

Added line #L180 was not covered by tests
}
chainConfig := getChainConfig(g.Blockchain, blkCtx.BlockHeight, evmNetworkID)

Expand Down Expand Up @@ -246,17 +246,17 @@
}
receipt := &action.Receipt{
GasConsumed: ps.gas - remainingGas,
BlockHeight: ps.blkCtx.BlockHeight,
ActionHash: ps.actionCtx.ActionHash,

Check warning on line 250 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L249-L250

Added lines #L249 - L250 were not covered by tests
ContractAddress: contractAddress,
}

receipt.Status = uint64(statusCode)
var (
depositLog, burnLog *action.TransactionLog
consumedGas = depositGas - remainingGas
)
if ps.featureCtx.FixDoubleChargeGas {

Check warning on line 259 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L255-L259

Added lines #L255 - L259 were not covered by tests
// 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))
} else {
Expand All @@ -264,37 +264,40 @@
remainingValue := new(big.Int).Mul(new(big.Int).SetUint64(remainingGas), ps.txCtx.GasPrice)
stateDB.AddBalance(ps.txCtx.Origin, remainingValue)
}
if consumedGas > 0 {

Check warning on line 267 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L267

Added line #L267 was not covered by tests
burnLog = &action.TransactionLog{
Type: iotextypes.TransactionLogType_GAS_FEE,
Sender: ps.actionCtx.Caller.String(),

Check warning on line 270 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L270

Added line #L270 was not covered by tests
Recipient: "", // burned
Amount: new(big.Int).Mul(new(big.Int).SetUint64(consumedGas), ps.txCtx.GasPrice),

Check warning on line 272 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L272

Added line #L272 was not covered by tests
}
}
}
if consumedGas > 0 {
var (
receiver address.Address
sharedGas uint64
sharedGasFee, totalGasFee *big.Int
)
if ps.featureCtx.SharedGasWithDapp && sgd != nil {

Check warning on line 282 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L276-L282

Added lines #L276 - L282 were not covered by tests
// TODO: sgd is whether nil should be checked in processSGD
receiver, sharedGas, err = processSGD(ctx, sm, execution, consumedGas, sgd)
if err != nil {

Check warning on line 285 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L284-L285

Added lines #L284 - L285 were not covered by tests
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)

Check warning on line 291 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L289-L291

Added lines #L289 - L291 were not covered by tests
}
totalGasFee = new(big.Int).Mul(new(big.Int).SetUint64(consumedGas), ps.txCtx.GasPrice)
depositLog, err = ps.helperCtx.DepositGasFunc(ctx, sm, receiver, totalGasFee, sharedGasFee)
if err != nil {
return nil, nil, err
if ps.helperCtx.DepositGasFunc != nil {
depositLog, err = ps.helperCtx.DepositGasFunc(ctx, sm, receiver, totalGasFee, sharedGasFee)
if err != nil {

Check warning on line 296 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L293-L296

Added lines #L293 - L296 were not covered by tests
return nil, nil, err
}
}

}

if err := stateDB.CommitContracts(); err != nil {
Expand All @@ -302,12 +305,12 @@
}
receipt.AddLogs(stateDB.Logs()...).AddTransactionLogs(depositLog, burnLog)
if receipt.Status == uint64(iotextypes.ReceiptStatus_Success) ||
ps.featureCtx.AddOutOfGasToTransactionLog && receipt.Status == uint64(iotextypes.ReceiptStatus_ErrCodeStoreOutOfGas) {

Check warning on line 308 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L308

Added line #L308 was not covered by tests
receipt.AddTransactionLogs(stateDB.TransactionLogs()...)
}
stateDB.clear()

if ps.featureCtx.SetRevertMessageToReceipt && receipt.Status == uint64(iotextypes.ReceiptStatus_ErrExecutionReverted) && retval != nil && bytes.Equal(retval[:4], _revertSelector) {

Check warning on line 313 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L313

Added line #L313 was not covered by tests
// in case of the execution revert error, parse the retVal and add to receipt
data := retval[4:]
msgLength := byteutil.BytesToUint64BigEndian(data[56:64])
Expand All @@ -319,24 +322,24 @@
}

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

Check warning on line 327 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L325-L327

Added lines #L325 - L327 were not covered by tests
}
height, err := sm.Height()
if err != nil {

Check warning on line 330 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L329-L330

Added lines #L329 - L330 were not covered by tests
return nil, 0, err
}
receiver, percentage, ok, err := sgd.CheckContract(ctx, execution.Contract(), height-1)
if err != nil || !ok {
return nil, 0, err

Check warning on line 335 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L333-L335

Added lines #L333 - L335 were not covered by tests
}

sharedGas := consumedGas * percentage / 100
if sharedGas > consumedGas {
sharedGas = consumedGas

Check warning on line 340 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L338-L340

Added lines #L338 - L340 were not covered by tests
}
return receiver, sharedGas, nil

Check warning on line 342 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L342

Added line #L342 was not covered by tests
}

// ReadContractStorage reads contract's storage
Expand Down Expand Up @@ -438,7 +441,7 @@
var (
accessList types.AccessList
)
evm := vm.NewEVM(evmParams.context, evmParams.txCtx, stateDB, chainConfig, evmParams.evmConfig)

Check warning on line 444 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L444

Added line #L444 was not covered by tests
if g.IsOkhotsk(blockHeight) {
accessList = evmParams.accessList
}
Expand Down Expand Up @@ -502,7 +505,7 @@
// the tx is reverted. After Okhotsk height, it is fixed inside RevertToSnapshot()
var (
deltaRefundByDynamicGas = evm.DeltaRefundByDynamicGas
featureCtx = evmParams.featureCtx

Check warning on line 508 in action/protocol/execution/evm/evm.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/execution/evm/evm.go#L508

Added line #L508 was not covered by tests
)
if !featureCtx.CorrectGasRefund && deltaRefundByDynamicGas != 0 {
if deltaRefundByDynamicGas > 0 {
Expand Down Expand Up @@ -639,15 +642,6 @@
)

ctx = protocol.WithFeatureCtx(ctx)
// TODO: move the logic out of SimulateExecution
helperCtx := mustGetHelperCtx(ctx)
ctx = WithHelperCtx(ctx, HelperContext{
GetBlockHash: helperCtx.GetBlockHash,
DepositGasFunc: func(context.Context, protocol.StateManager, address.Address, *big.Int, *big.Int) (*action.TransactionLog, error) {
return nil, nil
},
Sgd: nil,
})
Comment on lines -642 to -650
Copy link
Member

Choose a reason for hiding this comment

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

After moving this logic out, need to update on all the calling places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed: add default depositGasFunc when ps.helperCtx.DepositGasFunc not set

return ExecuteContract(
ctx,
sm,
Expand Down
14 changes: 12 additions & 2 deletions api/coreservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
accountutil "github.com/iotexproject/iotex-core/action/protocol/account/util"
"github.com/iotexproject/iotex-core/action/protocol/execution/evm"
"github.com/iotexproject/iotex-core/action/protocol/poll"
"github.com/iotexproject/iotex-core/action/protocol/rewarding"
"github.com/iotexproject/iotex-core/action/protocol/rolldpos"
"github.com/iotexproject/iotex-core/actpool"
logfilter "github.com/iotexproject/iotex-core/api/logfilter"
Expand Down Expand Up @@ -182,6 +183,7 @@
readCache *ReadCache
messageBatcher *batch.Manager
apiStats *nodestats.APILocalStats
sgdIndexer blockindex.SGDRegistry
}

// jobDesc provides a struct to get and store logs in core.LogsInRange
Expand Down Expand Up @@ -212,12 +214,19 @@
}

// WithAPIStats is the option to return RPC stats through API.
func WithAPIStats(stats *nodestats.APILocalStats) Option {
return func(svr *coreService) {
svr.apiStats = stats

Check warning on line 219 in api/coreservice.go

View check run for this annotation

Codecov / codecov/patch

api/coreservice.go#L217-L219

Added lines #L217 - L219 were not covered by tests
}
}

// WithSGDIndexer is the option to return SGD Indexer through API.
func WithSGDIndexer(sgdIndexer blockindex.SGDRegistry) Option {
return func(svr *coreService) {
svr.sgdIndexer = sgdIndexer

Check warning on line 226 in api/coreservice.go

View check run for this annotation

Codecov / codecov/patch

api/coreservice.go#L224-L226

Added lines #L224 - L226 were not covered by tests
}
}

type intrinsicGasCalculator interface {
IntrinsicGas() (uint64, error)
}
Expand Down Expand Up @@ -270,8 +279,8 @@

if core.broadcastHandler != nil {
core.messageBatcher = batch.NewManager(func(msg *batch.Message) error {
return core.broadcastHandler(context.Background(), core.bc.ChainID(), msg.Data)
})

Check warning on line 283 in api/coreservice.go

View check run for this annotation

Codecov / codecov/patch

api/coreservice.go#L282-L283

Added lines #L282 - L283 were not covered by tests
}

return &core, nil
Expand Down Expand Up @@ -488,7 +497,7 @@
return status.Errorf(codes.InvalidArgument, "ChainID does not match, expecting %d, got %d", core.bc.ChainID(), chainID)
}
if ge.IsMidway(core.bc.TipHeight()) && chainID != core.bc.ChainID() && chainID != 0 {
return status.Errorf(codes.InvalidArgument, "ChainID does not match, expecting %d, got %d", core.bc.ChainID(), chainID)

Check warning on line 500 in api/coreservice.go

View check run for this annotation

Codecov / codecov/patch

api/coreservice.go#L500

Added line #L500 was not covered by tests
}
return nil
}
Expand Down Expand Up @@ -1690,7 +1699,7 @@
return nil, nil, nil, err
}
if gasLimit == 0 {
gasLimit = core.bc.Genesis().BlockGasLimit

Check warning on line 1702 in api/coreservice.go

View check run for this annotation

Codecov / codecov/patch

api/coreservice.go#L1702

Added line #L1702 was not covered by tests
}
traces := logger.NewStructLogger(config)
ctx = protocol.WithVMConfigCtx(ctx, vm.Config{
Expand Down Expand Up @@ -1721,7 +1730,7 @@
return nil, nil, nil, err
}
getblockHash := func(height uint64) (hash.Hash256, error) {
return blkHash, nil

Check warning on line 1733 in api/coreservice.go

View check run for this annotation

Codecov / codecov/patch

api/coreservice.go#L1733

Added line #L1733 was not covered by tests
}
retval, receipt, err := core.simulateExecution(ctx, callerAddr, exec, getblockHash)
return retval, receipt, traces, err
Expand All @@ -1732,18 +1741,19 @@
if core.apiStats == nil {
return
}
elapsed := time.Since(start)
core.apiStats.ReportCall(nodestats.APIReport{
Method: method,
HandlingTime: elapsed,
Success: success,
}, size)

Check warning on line 1749 in api/coreservice.go

View check run for this annotation

Codecov / codecov/patch

api/coreservice.go#L1744-L1749

Added lines #L1744 - L1749 were not covered by tests
}

func (core *coreService) simulateExecution(ctx context.Context, addr address.Address, exec *action.Execution, getBlockHash evm.GetBlockHash) ([]byte, *action.Receipt, error) {
// TODO: add depositGas
ctx = evm.WithHelperCtx(ctx, evm.HelperContext{
GetBlockHash: getBlockHash,
GetBlockHash: getBlockHash,
DepositGasFunc: rewarding.DepositGasWithSGD,
Sgd: core.sgdIndexer,
Copy link
Member

Choose a reason for hiding this comment

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

will panic without WithSGDIndexer()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

})
return core.sf.SimulateExecution(ctx, addr, exec)
}
1 change: 1 addition & 0 deletions chainservice/chainservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ func (cs *ChainService) NewAPIServer(cfg api.Config, plugins map[int]interface{}
}),
api.WithNativeElection(cs.electionCommittee),
api.WithAPIStats(cs.apiStats),
api.WithSGDIndexer(cs.sgdIndexer),
}

svr, err := api.NewServerV2(
Expand Down
Loading