Skip to content

Commit

Permalink
fix: edge case with unprocessable requests
Browse files Browse the repository at this point in the history
  • Loading branch information
jorgemmsilva committed Sep 7, 2023
1 parent c079de8 commit 9b96f54
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 7 deletions.
4 changes: 4 additions & 0 deletions packages/chain/mempool/mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/chain/mempool/mempool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
3 changes: 3 additions & 0 deletions packages/solo/mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
Expand Down
3 changes: 2 additions & 1 deletion packages/vm/core/blocklog/unprocessable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion packages/vm/core/testcore/custom_onledger_requests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions packages/vm/gas/feepolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down
3 changes: 3 additions & 0 deletions packages/vm/vmimpl/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions packages/vm/vmimpl/runreq.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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)
}
Expand Down

0 comments on commit 9b96f54

Please sign in to comment.