From 9b96f54c4befc6ae6c544fb4b3b23a5befc99429 Mon Sep 17 00:00:00 2001 From: Jorge Silva Date: Thu, 7 Sep 2023 14:22:25 +0100 Subject: [PATCH] fix: edge case with unprocessable requests --- packages/chain/mempool/mempool.go | 4 ++++ packages/chain/mempool/mempool_test.go | 2 +- packages/solo/mempool.go | 3 +++ packages/vm/core/blocklog/unprocessable.go | 3 ++- .../vm/core/testcore/custom_onledger_requests_test.go | 2 +- packages/vm/gas/feepolicy.go | 3 +++ packages/vm/vmimpl/internal.go | 3 +++ packages/vm/vmimpl/runreq.go | 10 ++++++---- 8 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/chain/mempool/mempool.go b/packages/chain/mempool/mempool.go index 1885e4f7b3..531cad1c58 100644 --- a/packages/chain/mempool/mempool.go +++ b/packages/chain/mempool/mempool.go @@ -649,6 +649,10 @@ func (mpi *mempoolImpl) handleReceiveOnLedgerRequest(request isc.OnLedgerRequest mpi.log.Warnf("dropping request, because it has ReturnAmount, ID=%v", requestID) return } + if request.SenderAccount() == nil { + // do not process requests without the sender feature + return + } // // Check, maybe mempool already has it. if mpi.onLedgerPool.Has(requestRef) || mpi.timePool.Has(requestRef) { diff --git a/packages/chain/mempool/mempool_test.go b/packages/chain/mempool/mempool_test.go index 1400230a24..92109ad891 100644 --- a/packages/chain/mempool/mempool_test.go +++ b/packages/chain/mempool/mempool_test.go @@ -779,7 +779,7 @@ func getRequestsOnLedger(t *testing.T, chainAddress iotago.Address, amount int, } output := transaction.BasicOutputFromPostData( tpkg.RandEd25519Address(), - isc.Hn("dummySenderContract"), + 0, requestParams, ) outputID := tpkg.RandOutputID(uint16(i)) diff --git a/packages/solo/mempool.go b/packages/solo/mempool.go index cfbd7ef7a1..f130ace60d 100644 --- a/packages/solo/mempool.go +++ b/packages/solo/mempool.go @@ -49,6 +49,9 @@ func (mi *mempoolImpl) ReceiveRequests(reqs ...isc.Request) { mi.mu.Lock() defer mi.mu.Unlock() for _, req := range reqs { + if req.SenderAccount() == nil { + continue // ignore requests without a sender + } if _, ok := mi.requests[req.ID()]; !ok { mi.info.TotalPool++ mi.info.InPoolCounter++ diff --git a/packages/vm/core/blocklog/unprocessable.go b/packages/vm/core/blocklog/unprocessable.go index cd44160389..ec6aa2f058 100644 --- a/packages/vm/core/blocklog/unprocessable.go +++ b/packages/vm/core/blocklog/unprocessable.go @@ -130,7 +130,8 @@ func retryUnprocessable(ctx isc.Sandbox) dict.Dict { if err != nil { panic(ErrUnprocessableUnexpected) } - if !rec.SenderAccount().Equals(ctx.Request().SenderAccount()) { + recSender := rec.SenderAccount() + if rec.SenderAccount() == nil || !recSender.Equals(ctx.Request().SenderAccount()) { panic(ErrUnprocessableWrongSender) } ctx.Privileged().RetryUnprocessable(rec, outputID) diff --git a/packages/vm/core/testcore/custom_onledger_requests_test.go b/packages/vm/core/testcore/custom_onledger_requests_test.go index dd2e71ecda..375c01c0b1 100644 --- a/packages/vm/core/testcore/custom_onledger_requests_test.go +++ b/packages/vm/core/testcore/custom_onledger_requests_test.go @@ -106,7 +106,7 @@ func TestNoSenderFeature(t *testing.T) { reqs, err := ch.Env.RequestsForChain(tx, ch.ChainID) require.NoError(ch.Env.T, err) - results := ch.RunRequestsSync(reqs, "post") + results := ch.RunRequestsSync(reqs, "post") // under normal circumstances this request won't reach the mempool require.Len(t, results, 1) require.NotNil(t, results[0].Receipt.Error) err = ch.ResolveVMError(results[0].Receipt.Error) diff --git a/packages/vm/gas/feepolicy.go b/packages/vm/gas/feepolicy.go index 607ef6dbad..619d932b0b 100644 --- a/packages/vm/gas/feepolicy.go +++ b/packages/vm/gas/feepolicy.go @@ -60,6 +60,9 @@ func (p *FeePolicy) FeeFromGas(gasUnits uint64) uint64 { } func (p *FeePolicy) MinFee() uint64 { + if p.GasPerToken.A == 0 { + return 0 + } return p.FeeFromGas(BurnCodeMinimumGasPerRequest1P.Cost()) } diff --git a/packages/vm/vmimpl/internal.go b/packages/vm/vmimpl/internal.go index dc0849d3af..c9874ca89d 100644 --- a/packages/vm/vmimpl/internal.go +++ b/packages/vm/vmimpl/internal.go @@ -184,6 +184,9 @@ func (vmctx *vmContext) storeUnprocessable(chainState kv.KVStore, unprocessable withContractState(chainState, blocklog.Contract, func(s kv.KVStore) { for _, r := range unprocessable { + if r.SenderAccount() == nil { + continue + } txsnapshot := vmctx.createTxBuilderSnapshot() err := panicutil.CatchPanic(func() { position := vmctx.txbuilder.ConsumeUnprocessable(r) diff --git a/packages/vm/vmimpl/runreq.go b/packages/vm/vmimpl/runreq.go index 83bd01b027..5def7e3c01 100644 --- a/packages/vm/vmimpl/runreq.go +++ b/packages/vm/vmimpl/runreq.go @@ -98,15 +98,16 @@ func (reqctx *requestContext) creditAssetsToChain() { storageDepositNeeded := reqctx.vm.txbuilder.Consume(req) // if sender is specified, all assets goes to sender's sender - // Otherwise it all goes to the common sender and panics is logged in the SC call + // Otherwise it all goes to the common sender and panic is logged in the SC call sender := req.SenderAccount() if sender == nil { + if storageDepositNeeded > req.Assets().BaseTokens { + panic(vmexceptions.ErrNotEnoughFundsForSD) // if sender is not specified, and extra tokens are needed to pay for SD, the request cannot be processed. + } // onleger request with no sender, send all assets to the payoutAddress payoutAgentID := reqctx.vm.payoutAgentID() creditNFTToAccount(reqctx.uncommittedState, payoutAgentID, req) creditToAccount(reqctx.uncommittedState, payoutAgentID, req.Assets()) - - // debit any SD required for accounting UTXOs if storageDepositNeeded > 0 { debitFromAccount(reqctx.uncommittedState, payoutAgentID, isc.NewAssetsBaseTokens(storageDepositNeeded)) } @@ -115,7 +116,8 @@ func (reqctx *requestContext) creditAssetsToChain() { senderBaseTokens := req.Assets().BaseTokens + reqctx.GetBaseTokensBalance(sender) - if senderBaseTokens < storageDepositNeeded { + minReqCost := reqctx.ChainInfo().GasFeePolicy.MinFee() + if senderBaseTokens < storageDepositNeeded+minReqCost { // TODO why the weird issue when minReqCost is not present? // user doesn't have enough funds to pay for the SD needs of this request panic(vmexceptions.ErrNotEnoughFundsForSD) }