From 18b7900216e4458feb55d55ed1fa2f9fcf239a92 Mon Sep 17 00:00:00 2001 From: ZhangTao1596 Date: Thu, 9 Feb 2023 16:11:50 +0800 Subject: [PATCH 1/5] core: fix #2845 system fee refund attribute --- pkg/core/blockchain.go | 22 +++++++++-- pkg/core/interop/context.go | 32 ++++++++-------- pkg/core/native/native_gas.go | 9 +++++ pkg/core/native/policy.go | 41 +++++++++++++++++++-- pkg/core/transaction/attribute.go | 7 +++- pkg/core/transaction/attrtype.go | 11 +++--- pkg/core/transaction/refundablesystemfee.go | 20 ++++++++++ pkg/services/rpcsrv/server.go | 4 ++ 8 files changed, 118 insertions(+), 28 deletions(-) create mode 100644 pkg/core/transaction/refundablesystemfee.go diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index b56f68b134..bdaa3b165d 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1418,6 +1418,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error cache = bc.dao.GetPrivate() aerCache = bc.dao.GetPrivate() appExecResults = make([]*state.AppExecResult, 0, 2+len(block.Transactions)) + txesConsumed = make(map[util.Uint256]int64, len(block.Transactions)) aerchan = make(chan *state.AppExecResult, len(block.Transactions)/8) // Tested 8 and 4 with no practical difference, but feel free to test more and tune. aerdone = make(chan error) ) @@ -1500,7 +1501,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error close(aerdone) }() _ = cache.GetItemCtx() // Prime serialization context cache (it'll be reused by upper layer DAOs). - aer, v, err := bc.runPersist(bc.contracts.GetPersistScript(), block, cache, trigger.OnPersist, nil) + aer, v, err := bc.runPersist(bc.contracts.GetPersistScript(), block, cache, trigger.OnPersist, nil, nil) if err != nil { // Release goroutines, don't care about errors, we already have one. close(aerchan) @@ -1533,6 +1534,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error zap.Error(err)) faultException = err.Error() } + txesConsumed[tx.Hash()] = v.GasConsumed() aer := &state.AppExecResult{ Container: tx.Hash(), Execution: state.Execution{ @@ -1548,7 +1550,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error aerchan <- aer } - aer, _, err = bc.runPersist(bc.contracts.GetPostPersistScript(), block, cache, trigger.PostPersist, v) + aer, _, err = bc.runPersist(bc.contracts.GetPostPersistScript(), block, cache, trigger.PostPersist, v, txesConsumed) if err != nil { // Release goroutines, don't care about errors, we already have one. close(aerchan) @@ -1683,13 +1685,14 @@ func (bc *Blockchain) IsExtensibleAllowed(u util.Uint160) bool { return n < len(us) } -func (bc *Blockchain) runPersist(script []byte, block *block.Block, cache *dao.Simple, trig trigger.Type, v *vm.VM) (*state.AppExecResult, *vm.VM, error) { +func (bc *Blockchain) runPersist(script []byte, block *block.Block, cache *dao.Simple, trig trigger.Type, v *vm.VM, txesConsumed map[util.Uint256]int64) (*state.AppExecResult, *vm.VM, error) { systemInterop := bc.newInteropContext(trig, cache, block, nil) if v == nil { v = systemInterop.SpawnVM() } else { systemInterop.ReuseVM(v) } + systemInterop.TxesConsumed = txesConsumed v.LoadScriptWithFlags(script, callflag.All) if err := systemInterop.Exec(); err != nil { return nil, v, fmt.Errorf("VM has failed: %w", err) @@ -2248,6 +2251,11 @@ func (bc *Blockchain) FeePerByte() int64 { return bc.contracts.Policy.GetFeePerByteInternal(bc.dao) } +// GasRefundFee returns extra fee for system fee refundable transaction +func (bc *Blockchain) SystemFeeRefundCost() int64 { + return bc.contracts.Policy.GetSystemFeeRefundCostInternal(bc.dao) +} + // GetMemPool returns the memory pool of the blockchain. func (bc *Blockchain) GetMemPool() *mempool.Pool { return bc.memPool @@ -2367,6 +2375,9 @@ func (bc *Blockchain) verifyAndPoolTx(t *transaction.Transaction, pool *mempool. needNetworkFee += (int64(na.NKeys) + 1) * bc.contracts.Notary.GetNotaryServiceFeePerKey(bc.dao) } } + if len(t.GetAttributes(transaction.RefundableSystemFeeT)) > 0 { + needNetworkFee += bc.SystemFeeRefundCost() + } netFee := t.NetworkFee - needNetworkFee if netFee < 0 { return fmt.Errorf("%w: net fee is %v, need %v", ErrTxSmallNetworkFee, t.NetworkFee, needNetworkFee) @@ -2480,6 +2491,11 @@ func (bc *Blockchain) verifyTxAttributes(d *dao.Simple, tx *transaction.Transact if !tx.HasSigner(bc.contracts.Notary.Hash) { return fmt.Errorf("%w: NotaryAssisted attribute was found, but transaction is not signed by the Notary native contract", ErrInvalidAttribute) } + case transaction.RefundableSystemFeeT: + state := bc.GetContractState(tx.Sender()) + if state != nil { + return fmt.Errorf("%w: RefundableSystemFee attribute was found, but transaction sender is contract", ErrInvalidAttribute) + } default: if !bc.config.ReservedAttributes && attrType >= transaction.ReservedLowerBound && attrType <= transaction.ReservedUpperBound { return fmt.Errorf("%w: attribute of reserved type was found, but ReservedAttributes are disabled", ErrInvalidAttribute) diff --git a/pkg/core/interop/context.go b/pkg/core/interop/context.go index 1d1448014f..c244950f2e 100644 --- a/pkg/core/interop/context.go +++ b/pkg/core/interop/context.go @@ -45,21 +45,23 @@ type Ledger interface { // Context represents context in which interops are executed. type Context struct { - Chain Ledger - Container hash.Hashable - Network uint32 - Hardforks map[string]uint32 - Natives []Contract - Trigger trigger.Type - Block *block.Block - NonceData [16]byte - Tx *transaction.Transaction - DAO *dao.Simple - Notifications []state.NotificationEvent - Log *zap.Logger - VM *vm.VM - Functions []Function - Invocations map[util.Uint160]int + Chain Ledger + Container hash.Hashable + Network uint32 + Hardforks map[string]uint32 + Natives []Contract + Trigger trigger.Type + Block *block.Block + NonceData [16]byte + Tx *transaction.Transaction + DAO *dao.Simple + Notifications []state.NotificationEvent + Log *zap.Logger + VM *vm.VM + Functions []Function + Invocations map[util.Uint160]int + TxesConsumed map[util.Uint256]int64 + cancelFuncs []context.CancelFunc getContract func(*dao.Simple, util.Uint160) (*state.Contract, error) baseExecFee int64 diff --git a/pkg/core/native/native_gas.go b/pkg/core/native/native_gas.go index e60f3f9595..8357269479 100644 --- a/pkg/core/native/native_gas.go +++ b/pkg/core/native/native_gas.go @@ -129,6 +129,15 @@ func (g *GAS) OnPersist(ic *interop.Context) error { // PostPersist implements the Contract interface. func (g *GAS) PostPersist(ic *interop.Context) error { + for _, tx := range ic.Block.Transactions { + attrs := tx.GetAttributes(transaction.RefundableSystemFeeT) + if len(attrs) != 0 { + consumed := ic.TxesConsumed[tx.Hash()] + if consumed < tx.SystemFee { + g.mint(ic, tx.Sender(), big.NewInt(tx.SystemFee-consumed), false) + } + } + } return nil } diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 570dfc40a6..59dcf374c1 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -21,9 +21,10 @@ import ( const ( policyContractID = -7 - defaultExecFeeFactor = interop.DefaultBaseExecFee - defaultFeePerByte = 1000 - defaultMaxVerificationGas = 1_50000000 + defaultExecFeeFactor = interop.DefaultBaseExecFee + defaultFeePerByte = 1000 + defaultMaxVerificationGas = 1_50000000 + defaultSystemFeeRefundCost = 0_10000000 // DefaultStoragePrice is the price to pay for 1 byte of storage. DefaultStoragePrice = 100000 @@ -33,7 +34,8 @@ const ( maxFeePerByte = 100_000_000 // maxStoragePrice is the maximum allowed price for a byte of storage. maxStoragePrice = 10000000 - + // maxSystemFeeRefundCost is the maximun allowed extra fee for gas refundable transaction + maxSystemFeeRefundCost = 1_00000000 // blockedAccountPrefix is a prefix used to store blocked account. blockedAccountPrefix = 15 ) @@ -46,6 +48,8 @@ var ( feePerByteKey = []byte{10} // storagePriceKey is a key used to store storage price. storagePriceKey = []byte{19} + // systemFeeRefundCostKey is a key usesd to store gas refund fee + systemFeeRefundCostKey = []byte{20} ) // Policy represents Policy native contract. @@ -59,6 +63,7 @@ type PolicyCache struct { feePerByte int64 maxVerificationGas int64 storagePrice uint32 + systemFeeRefundCost int64 blockedAccounts []util.Uint160 } @@ -127,6 +132,11 @@ func newPolicy() *Policy { md = newMethodAndPrice(p.unblockAccount, 1<<15, callflag.States) p.AddMethod(md, desc) + desc = newDescriptor("setSystemFeeRefundCost", smartcontract.VoidType, + manifest.NewParameter("value", smartcontract.IntegerType)) + md = newMethodAndPrice(p.setSystemFeeRefundCost, 1<<15, callflag.States) + p.AddMethod(md, desc) + return p } @@ -140,12 +150,14 @@ func (p *Policy) Initialize(ic *interop.Context) error { setIntWithKey(p.ID, ic.DAO, feePerByteKey, defaultFeePerByte) setIntWithKey(p.ID, ic.DAO, execFeeFactorKey, defaultExecFeeFactor) setIntWithKey(p.ID, ic.DAO, storagePriceKey, DefaultStoragePrice) + setIntWithKey(p.ID, ic.DAO, systemFeeRefundCostKey, defaultSystemFeeRefundCost) cache := &PolicyCache{ execFeeFactor: defaultExecFeeFactor, feePerByte: defaultFeePerByte, maxVerificationGas: defaultMaxVerificationGas, storagePrice: DefaultStoragePrice, + systemFeeRefundCost: defaultSystemFeeRefundCost, blockedAccounts: make([]util.Uint160, 0), } ic.DAO.SetCache(p.ID, cache) @@ -168,6 +180,7 @@ func (p *Policy) fillCacheFromDAO(cache *PolicyCache, d *dao.Simple) error { cache.feePerByte = getIntWithKey(p.ID, d, feePerByteKey) cache.maxVerificationGas = defaultMaxVerificationGas cache.storagePrice = uint32(getIntWithKey(p.ID, d, storagePriceKey)) + cache.systemFeeRefundCost = getIntWithKey(p.ID, d, systemFeeRefundCostKey) cache.blockedAccounts = make([]util.Uint160, 0) var fErr error @@ -354,6 +367,26 @@ func (p *Policy) unblockAccount(ic *interop.Context, args []stackitem.Item) stac return stackitem.NewBool(true) } +func (p *Policy) GetSystemFeeRefundCostInternal(d *dao.Simple) int64 { + cache := d.GetROCache(p.ID).(*PolicyCache) + return cache.systemFeeRefundCost +} + +// setSystemFeeRefundCost is a Policy contract method that set extra network fee for gas refundable transaction. +func (p *Policy) setSystemFeeRefundCost(ic *interop.Context, args []stackitem.Item) stackitem.Item { + value := toBigInt(args[0]).Int64() + if value < 0 || value > maxSystemFeeRefundCost { + panic(fmt.Errorf("SystemFeeRefundCost shouldn't be negative or greater than %d", maxSystemFeeRefundCost)) + } + if !p.NEO.checkCommittee(ic) { + panic("invalid committee signature") + } + setIntWithKey(p.ID, ic.DAO, systemFeeRefundCostKey, value) + cache := ic.DAO.GetRWCache(p.ID).(*PolicyCache) + cache.systemFeeRefundCost = value + return stackitem.Null{} +} + // CheckPolicy checks whether a transaction conforms to the current policy restrictions, // like not being signed by a blocked account or not exceeding the block-level system // fee limit. diff --git a/pkg/core/transaction/attribute.go b/pkg/core/transaction/attribute.go index 91f8b66e59..446a6c26b0 100644 --- a/pkg/core/transaction/attribute.go +++ b/pkg/core/transaction/attribute.go @@ -41,6 +41,8 @@ func (attr *Attribute) DecodeBinary(br *io.BinReader) { attr.Value = new(Conflicts) case NotaryAssistedT: attr.Value = new(NotaryAssisted) + case RefundableSystemFeeT: + attr.Value = new(RefundableSystemFee) default: if t >= ReservedLowerBound && t <= ReservedUpperBound { attr.Value = new(Reserved) @@ -57,7 +59,7 @@ func (attr *Attribute) EncodeBinary(bw *io.BinWriter) { bw.WriteB(byte(attr.Type)) switch t := attr.Type; t { case HighPriority: - case OracleResponseT, NotValidBeforeT, ConflictsT, NotaryAssistedT: + case OracleResponseT, NotValidBeforeT, ConflictsT, NotaryAssistedT, RefundableSystemFeeT: attr.Value.EncodeBinary(bw) default: if t >= ReservedLowerBound && t <= ReservedUpperBound { @@ -102,6 +104,9 @@ func (attr *Attribute) UnmarshalJSON(data []byte) error { case NotaryAssistedT.String(): attr.Type = NotaryAssistedT attr.Value = new(NotaryAssisted) + case RefundableSystemFeeT.String(): + attr.Type = RefundableSystemFeeT + attr.Value = new(RefundableSystemFee) default: return errors.New("wrong Type") } diff --git a/pkg/core/transaction/attrtype.go b/pkg/core/transaction/attrtype.go index c7a57f14c8..e1bf08e5c9 100644 --- a/pkg/core/transaction/attrtype.go +++ b/pkg/core/transaction/attrtype.go @@ -14,11 +14,12 @@ const ( // List of valid attribute types. const ( - HighPriority AttrType = 1 - OracleResponseT AttrType = 0x11 // OracleResponse - NotValidBeforeT AttrType = 0x20 // NotValidBefore - ConflictsT AttrType = 0x21 // Conflicts - NotaryAssistedT AttrType = 0x22 // NotaryAssisted + HighPriority AttrType = 1 + OracleResponseT AttrType = 0x11 // OracleResponse + NotValidBeforeT AttrType = 0x20 // NotValidBefore + ConflictsT AttrType = 0x21 // Conflicts + NotaryAssistedT AttrType = 0x22 // NotaryAssisted + RefundableSystemFeeT AttrType = 0x30 // RefundableSystemFee ) func (a AttrType) allowMultiple() bool { diff --git a/pkg/core/transaction/refundablesystemfee.go b/pkg/core/transaction/refundablesystemfee.go new file mode 100644 index 0000000000..a836821663 --- /dev/null +++ b/pkg/core/transaction/refundablesystemfee.go @@ -0,0 +1,20 @@ +package transaction + +import ( + "github.com/nspcc-dev/neo-go/pkg/io" +) + +// Conflicts represents attribute for refund gas transaction. +type RefundableSystemFee struct { +} + +// DecodeBinary implements the io.Serializable interface. +func (c *RefundableSystemFee) DecodeBinary(br *io.BinReader) { +} + +// EncodeBinary implements the io.Serializable interface. +func (c *RefundableSystemFee) EncodeBinary(w *io.BinWriter) { +} + +func (c *RefundableSystemFee) toJSONMap(m map[string]interface{}) { +} diff --git a/pkg/services/rpcsrv/server.go b/pkg/services/rpcsrv/server.go index e2a5d2a58d..0af17a8698 100644 --- a/pkg/services/rpcsrv/server.go +++ b/pkg/services/rpcsrv/server.go @@ -67,6 +67,7 @@ type ( CalculateClaimable(h util.Uint160, endHeight uint32) (*big.Int, error) CurrentBlockHash() util.Uint256 FeePerByte() int64 + SystemFeeRefundCost() int64 ForEachNEP11Transfer(acc util.Uint160, newestTimestamp uint64, f func(*state.NEP11Transfer) (bool, error)) error ForEachNEP17Transfer(acc util.Uint160, newestTimestamp uint64, f func(*state.NEP17Transfer) (bool, error)) error GetAppExecResults(util.Uint256, trigger.Type) ([]state.AppExecResult, error) @@ -847,6 +848,9 @@ func (s *Server) calculateNetworkFee(reqParams params.Params) (interface{}, *neo netFee += (int64(na.NKeys) + 1) * s.chain.GetNotaryServiceFeePerKey() } } + if len(tx.GetAttributes(transaction.RefundableSystemFeeT)) > 0 { + netFee += s.chain.SystemFeeRefundCost() + } fee := s.chain.FeePerByte() netFee += int64(size) * fee return result.NetworkFee{Value: netFee}, nil From 238f254b19364d9d6221fc4e9037afdba7e3ed09 Mon Sep 17 00:00:00 2001 From: ZhangTao1596 Date: Mon, 13 Feb 2023 14:30:52 +0800 Subject: [PATCH 2/5] core: fix #2845 Roman's feedback --- pkg/core/blockchain.go | 8 ++++---- pkg/core/interop/context.go | 2 +- pkg/core/native/native_gas.go | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index bdaa3b165d..9fbf6781e0 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1418,7 +1418,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error cache = bc.dao.GetPrivate() aerCache = bc.dao.GetPrivate() appExecResults = make([]*state.AppExecResult, 0, 2+len(block.Transactions)) - txesConsumed = make(map[util.Uint256]int64, len(block.Transactions)) + txesConsumed = make([]int64, len(block.Transactions)) aerchan = make(chan *state.AppExecResult, len(block.Transactions)/8) // Tested 8 and 4 with no practical difference, but feel free to test more and tune. aerdone = make(chan error) ) @@ -1511,7 +1511,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error appExecResults = append(appExecResults, aer) aerchan <- aer - for _, tx := range block.Transactions { + for i, tx := range block.Transactions { systemInterop := bc.newInteropContext(trigger.Application, cache, block, tx) systemInterop.ReuseVM(v) v.LoadScriptWithFlags(tx.Script, callflag.All) @@ -1534,7 +1534,7 @@ func (bc *Blockchain) storeBlock(block *block.Block, txpool *mempool.Pool) error zap.Error(err)) faultException = err.Error() } - txesConsumed[tx.Hash()] = v.GasConsumed() + txesConsumed[i] = v.GasConsumed() aer := &state.AppExecResult{ Container: tx.Hash(), Execution: state.Execution{ @@ -1685,7 +1685,7 @@ func (bc *Blockchain) IsExtensibleAllowed(u util.Uint160) bool { return n < len(us) } -func (bc *Blockchain) runPersist(script []byte, block *block.Block, cache *dao.Simple, trig trigger.Type, v *vm.VM, txesConsumed map[util.Uint256]int64) (*state.AppExecResult, *vm.VM, error) { +func (bc *Blockchain) runPersist(script []byte, block *block.Block, cache *dao.Simple, trig trigger.Type, v *vm.VM, txesConsumed []int64) (*state.AppExecResult, *vm.VM, error) { systemInterop := bc.newInteropContext(trig, cache, block, nil) if v == nil { v = systemInterop.SpawnVM() diff --git a/pkg/core/interop/context.go b/pkg/core/interop/context.go index c244950f2e..1a595007ce 100644 --- a/pkg/core/interop/context.go +++ b/pkg/core/interop/context.go @@ -60,7 +60,7 @@ type Context struct { VM *vm.VM Functions []Function Invocations map[util.Uint160]int - TxesConsumed map[util.Uint256]int64 + TxesConsumed []int64 cancelFuncs []context.CancelFunc getContract func(*dao.Simple, util.Uint160) (*state.Contract, error) diff --git a/pkg/core/native/native_gas.go b/pkg/core/native/native_gas.go index 8357269479..7f5a7b1974 100644 --- a/pkg/core/native/native_gas.go +++ b/pkg/core/native/native_gas.go @@ -129,10 +129,10 @@ func (g *GAS) OnPersist(ic *interop.Context) error { // PostPersist implements the Contract interface. func (g *GAS) PostPersist(ic *interop.Context) error { - for _, tx := range ic.Block.Transactions { + for i, tx := range ic.Block.Transactions { attrs := tx.GetAttributes(transaction.RefundableSystemFeeT) if len(attrs) != 0 { - consumed := ic.TxesConsumed[tx.Hash()] + consumed := ic.TxesConsumed[i] if consumed < tx.SystemFee { g.mint(ic, tx.Sender(), big.NewInt(tx.SystemFee-consumed), false) } From d8bcb2582ae7b3b324ddd1e58577152de6b5a21c Mon Sep 17 00:00:00 2001 From: ZhangTao1596 Date: Wed, 15 Feb 2023 10:35:32 +0800 Subject: [PATCH 3/5] core: fix #2845 attribute type string --- pkg/core/transaction/attrtype_string.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/core/transaction/attrtype_string.go b/pkg/core/transaction/attrtype_string.go index d3564aa4c2..f0fd80782c 100644 --- a/pkg/core/transaction/attrtype_string.go +++ b/pkg/core/transaction/attrtype_string.go @@ -13,12 +13,14 @@ func _() { _ = x[NotValidBeforeT-32] _ = x[ConflictsT-33] _ = x[NotaryAssistedT-34] + _ = x[RefundableSystemFeeT-48] } const ( _AttrType_name_0 = "HighPriority" _AttrType_name_1 = "OracleResponse" _AttrType_name_2 = "NotValidBeforeConflictsNotaryAssisted" + _AttrType_name_3 = "RefundableSystemFee" ) var ( @@ -34,6 +36,8 @@ func (i AttrType) String() string { case 32 <= i && i <= 34: i -= 32 return _AttrType_name_2[_AttrType_index_2[i]:_AttrType_index_2[i+1]] + case i == 48: + return _AttrType_name_3 default: return "AttrType(" + strconv.FormatInt(int64(i), 10) + ")" } From f9fb12a7df5aabd6ab1636ca687aa74a6fb86388 Mon Sep 17 00:00:00 2001 From: ZhangTao1596 Date: Wed, 1 Mar 2023 16:23:18 +0800 Subject: [PATCH 4/5] core: anna's feedback (fix #2845) --- pkg/core/native/policy.go | 33 +++++++++++++-------- pkg/core/transaction/refundablesystemfee.go | 2 +- pkg/interop/native/policy/policy.go | 10 +++++++ pkg/rpcclient/policy/policy.go | 19 ++++++++++-- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/pkg/core/native/policy.go b/pkg/core/native/policy.go index 59dcf374c1..aabeaec1d6 100644 --- a/pkg/core/native/policy.go +++ b/pkg/core/native/policy.go @@ -59,12 +59,12 @@ type Policy struct { } type PolicyCache struct { - execFeeFactor uint32 - feePerByte int64 - maxVerificationGas int64 - storagePrice uint32 - systemFeeRefundCost int64 - blockedAccounts []util.Uint160 + execFeeFactor uint32 + feePerByte int64 + maxVerificationGas int64 + storagePrice uint32 + systemFeeRefundCost int64 + blockedAccounts []util.Uint160 } var ( @@ -132,6 +132,11 @@ func newPolicy() *Policy { md = newMethodAndPrice(p.unblockAccount, 1<<15, callflag.States) p.AddMethod(md, desc) + desc = newDescriptor("getSystemFeeRefundCost", smartcontract.IntegerType, + manifest.NewParameter("value", smartcontract.IntegerType)) + md = newMethodAndPrice(p.GetSystemFeeRefundCost, 1<<15, callflag.ReadStates) + p.AddMethod(md, desc) + desc = newDescriptor("setSystemFeeRefundCost", smartcontract.VoidType, manifest.NewParameter("value", smartcontract.IntegerType)) md = newMethodAndPrice(p.setSystemFeeRefundCost, 1<<15, callflag.States) @@ -153,12 +158,12 @@ func (p *Policy) Initialize(ic *interop.Context) error { setIntWithKey(p.ID, ic.DAO, systemFeeRefundCostKey, defaultSystemFeeRefundCost) cache := &PolicyCache{ - execFeeFactor: defaultExecFeeFactor, - feePerByte: defaultFeePerByte, - maxVerificationGas: defaultMaxVerificationGas, - storagePrice: DefaultStoragePrice, - systemFeeRefundCost: defaultSystemFeeRefundCost, - blockedAccounts: make([]util.Uint160, 0), + execFeeFactor: defaultExecFeeFactor, + feePerByte: defaultFeePerByte, + maxVerificationGas: defaultMaxVerificationGas, + storagePrice: DefaultStoragePrice, + systemFeeRefundCost: defaultSystemFeeRefundCost, + blockedAccounts: make([]util.Uint160, 0), } ic.DAO.SetCache(p.ID, cache) @@ -367,6 +372,10 @@ func (p *Policy) unblockAccount(ic *interop.Context, args []stackitem.Item) stac return stackitem.NewBool(true) } +func (p *Policy) GetSystemFeeRefundCost(ic *interop.Context, args []stackitem.Item) stackitem.Item { + return stackitem.NewBigInteger(big.NewInt(p.GetSystemFeeRefundCostInternal(ic.DAO))) +} + func (p *Policy) GetSystemFeeRefundCostInternal(d *dao.Simple) int64 { cache := d.GetROCache(p.ID).(*PolicyCache) return cache.systemFeeRefundCost diff --git a/pkg/core/transaction/refundablesystemfee.go b/pkg/core/transaction/refundablesystemfee.go index a836821663..93dde5b7cd 100644 --- a/pkg/core/transaction/refundablesystemfee.go +++ b/pkg/core/transaction/refundablesystemfee.go @@ -4,7 +4,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/io" ) -// Conflicts represents attribute for refund gas transaction. +// RefundableSystemFee represents attribute for system fee refundable transaction. type RefundableSystemFee struct { } diff --git a/pkg/interop/native/policy/policy.go b/pkg/interop/native/policy/policy.go index b7aeeaca1b..b46b9efba8 100644 --- a/pkg/interop/native/policy/policy.go +++ b/pkg/interop/native/policy/policy.go @@ -57,3 +57,13 @@ func BlockAccount(addr interop.Hash160) bool { func UnblockAccount(addr interop.Hash160) bool { return neogointernal.CallWithToken(Hash, "unblockAccount", int(contract.States), addr).(bool) } + +// GetSystemFeeRefundableCost represents `getSystemFeeRefundCost` method of Policy native contract. +func GetSystemFeeRefundableCost() int { + return neogointernal.CallWithToken(Hash, "getSystemFeeRefundCost", int(contract.ReadStates)).(int) +} + +// SetSystemFeeRefundableCost represents `setSystemFeeRefundCost` method of Policy native contract. +func SetSystemFeeRefundableCost(value int) { + neogointernal.CallWithTokenNoRet(Hash, "setSystemFeeRefundCost", int(contract.States), value) +} diff --git a/pkg/rpcclient/policy/policy.go b/pkg/rpcclient/policy/policy.go index 2f583db0a7..6890aeabda 100644 --- a/pkg/rpcclient/policy/policy.go +++ b/pkg/rpcclient/policy/policy.go @@ -37,9 +37,10 @@ type Actor interface { var Hash = state.CreateNativeContractHash(nativenames.Policy) const ( - execFeeSetter = "setExecFeeFactor" - feePerByteSetter = "setFeePerByte" - storagePriceSetter = "setStoragePrice" + execFeeSetter = "setExecFeeFactor" + feePerByteSetter = "setFeePerByte" + storagePriceSetter = "setStoragePrice" + systemFeeRefundCostSetter = "setSystemFeeRefundCost" ) // ContractReader provides an interface to call read-only PolicyContract @@ -88,6 +89,12 @@ func (c *ContractReader) GetStoragePrice() (int64, error) { return unwrap.Int64(c.invoker.Call(Hash, "getStoragePrice")) } +// GetSystemFeeRefundCost returns extra network fee cost when using SystemFeeRefundableAttribute +// in transaction +func (c *ContractReader) GetSystemFeeRefundCost() (int64, error) { + return unwrap.Int64(c.invoker.Call(Hash, "getSystemFeeRefundCost")) +} + // IsBlocked checks if the given account is blocked in the PolicyContract. func (c *ContractReader) IsBlocked(account util.Uint160) (bool, error) { return unwrap.Bool(c.invoker.Call(Hash, "isBlocked", account)) @@ -158,6 +165,12 @@ func (c *Contract) SetStoragePriceUnsigned(value int64) (*transaction.Transactio return c.actor.MakeUnsignedCall(Hash, storagePriceSetter, nil, value) } +// SetSystemFeeRefundCost creates a transaction that sets the extra network fee cost in +// system fee refundable transaction. This transaction is not signed and just returned to the caller. +func (c *Contract) SetSystemFeeRefundCost(value int64) (*transaction.Transaction, error) { + return c.actor.MakeUnsignedCall(Hash, systemFeeRefundCostSetter, nil, value) +} + // BlockAccount creates and sends a transaction that blocks an account on the // network (via `blockAccount` method), it fails (with FAULT state) if it's not // successful. The returned values are transaction hash, its From 1cb4dda5e069820768dfdf61258fc01673244f8d Mon Sep 17 00:00:00 2001 From: ZhangTao1596 Date: Thu, 2 Mar 2023 17:30:18 +0800 Subject: [PATCH 5/5] core: system fee refund ut (fix #2845) --- pkg/core/blockchain_neotest_test.go | 57 ++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index f71b8890b8..bc611d0b65 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -451,6 +451,52 @@ func TestBlockchain_AddBadBlock(t *testing.T) { }) } +func TestBlockchain_RefundSystemFee(t *testing.T) { + bc, acc := chain.NewSingle(t) + e := neotest.NewExecutor(t, bc, acc, acc) + neoHash := e.NativeHash(t, nativenames.Neo) + + bal0 := bc.GetUtilityTokenBalance(acc.ScriptHash()) + + // tx without sysfee refund + tx1 := e.NewUnsignedTx(t, neoHash, "symbol") + e.SignTx(t, tx1, 100000000, acc) + vm, err := neotest.TestInvoke(bc, tx1) + actualSysfee1 := vm.GasConsumed() + require.NoError(t, err) + + block1 := e.NewUnsignedBlock(t, tx1) + e.SignBlock(block1) + err = bc.AddBlock(block1) + require.NoError(t, err) + bal1 := bc.GetUtilityTokenBalance(acc.ScriptHash()) + + // tx with sysfee refund + tx2 := e.NewUnsignedTx(t, neoHash, "symbol") + attr := transaction.Attribute{Type: transaction.RefundableSystemFeeT, Value: &transaction.RefundableSystemFee{}} + tx2.Attributes = append(tx2.Attributes, attr) + e.SignTx(t, tx2, 100000000, acc) + tx2.NetworkFee += bc.SystemFeeRefundCost() + vm, err = neotest.TestInvoke(bc, tx1) + actualSysfee2 := vm.GasConsumed() + require.NoError(t, err) + require.Equal(t, actualSysfee1, actualSysfee2) + + block2 := e.NewUnsignedBlock(t, tx2) + e.SignBlock(block2) + err = bc.AddBlock(block2) + require.NoError(t, err) + bal2 := bc.GetUtilityTokenBalance(acc.ScriptHash()) + + gas1 := new(big.Int).Sub(bal0, bal1) + gas2 := new(big.Int).Sub(bal1, bal2) + extraNetfee := big.NewInt(int64(io.GetVarSize(&attr)) * int64(bc.FeePerByte())) + extraNetfee.Add(extraNetfee, big.NewInt(bc.SystemFeeRefundCost())) + expect := new(big.Int).Add(gas1, extraNetfee) + expect.Sub(expect, big.NewInt(100000000-actualSysfee1)) + require.Equal(t, expect, gas2) +} + func TestBlockchain_GetHeader(t *testing.T) { bc, acc := chain.NewSingle(t) e := neotest.NewExecutor(t, bc, acc, acc) @@ -1109,7 +1155,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { } testScript := []byte{byte(opcode.PUSH1)} - newTestTx := func(t *testing.T, signer util.Uint160, script []byte) *transaction.Transaction { + newTestTx := func(t *testing.T, signer util.Uint160, script []byte, attrs ...transaction.Attribute) *transaction.Transaction { tx := transaction.New(script, 1_000_000) tx.Nonce = neotest.Nonce() tx.ValidUntilBlock = e.Chain.BlockHeight() + 5 @@ -1117,6 +1163,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { Account: signer, Scopes: transaction.CalledByEntry, }} + tx.Attributes = attrs tx.NetworkFee = int64(io.GetVarSize(tx)+200 /* witness */) * bc.FeePerByte() tx.NetworkFee += 1_000_000 // verification cost return tx @@ -1338,6 +1385,14 @@ func TestBlockchain_VerifyTx(t *testing.T) { }} require.NoError(t, bc.VerifyTx(tx)) }) + t.Run("SystemFeeRefundableAttribute", func(t *testing.T) { + tx := newTestTx(t, h, testScript, transaction.Attribute{Type: transaction.RefundableSystemFeeT, Value: &transaction.RefundableSystemFee{}}) + require.NoError(t, accs[0].SignTx(netmode.UnitTestNet, tx)) + checkErr(t, core.ErrTxSmallNetworkFee, tx) + tx.NetworkFee += bc.SystemFeeRefundCost() + require.Nil(t, bc.VerifyTx(tx)) + t.Log("success") + }) t.Run("Oracle", func(t *testing.T) { cs := contracts.GetOracleContractState(t, pathToInternalContracts, validator.ScriptHash(), 0) e.DeployContract(t, &neotest.Contract{