Expired NFT Offers aren't actually deleted (Version: 2.3.0)

[mvadari]

Issue Description

There is code that appears to be intended to delete NFT offers if they're expired. I assume that that correlates with this tecEXPIRED code in the NFTokenAcceptOffer code.

However, if you actually trigger tecEXPIRED in NFTokenAcceptOffer, it doesn't delete the NFT.

See https://testnet.xrpl.org/transactions/07C9D580608D754A69B812924B0C7965920E7120BBD0F0D469D60D9FFC3AB252/raw

Steps to Reproduce

const xrpl = require("xrpl");

const sleep = ms => new Promise(r => setTimeout(r, ms));

const client = new xrpl.Client("wss://s.altnet.rippletest.net:51233");

async function test() {
  await client.connect();
  console.log("connected");
  const {wallet} = await client.fundWallet();
  const {wallet: wallet2} = await client.fundWallet();

  const response = await client.submitAndWait({
    TransactionType: 'NFTokenMint',
    Account: wallet.address,
    NFTokenTaxon: 0,
    Flags: xrpl.NFTokenMintFlags.tfTransferable,
    URI: xrpl.convertStringToHex("nft_example"),
  }, {autofill: true, wallet})
  const nftoken_id = response.result.meta.nftoken_id
  console.log(nftoken_id)

  const close_time = (
    await client.request({
      command: 'ledger',
      ledger_index: 'validated',
    })
  ).result.ledger.close_time

  const response2 = await client.submitAndWait({
    TransactionType: 'NFTokenCreateOffer',
    Account: wallet.address,
    NFTokenID: nftoken_id,
    Amount: "1",
    Flags: 1,
    Expiration: close_time + 2,
  }, {autofill: true, wallet})
  console.log(JSON.stringify(response2.result, null, 4))

  const offerId = response2.result.meta.offer_id

  await sleep(5000)

  const response3 = await client.submitAndWait({
    TransactionType: 'NFTokenAcceptOffer',
    Account: wallet2.address,
    NFTokenSellOffer: offerId,
  }, {autofill: true, wallet: wallet2})
  console.log(JSON.stringify(response3.result, null, 4))

  console.log((await client.request({command: "account_objects", account: wallet.address})).result.account_objects)

  await client.disconnect()
}

test()

Expected Result

I expect the expired NFT offer to be deleted.

Actual Result

The expired NFT offer is not deleted.

This seems to be because the tecEXPIRED error is thrown in preclaim, not doApply, and the NFTs aren't actually deleted in the view, which is necessary for the deletion logic to work properly.

Note: there are tests that check for tecEXPIRED, but they don't actually do anything about checking that the offer is deleted.

[Bronek]

Fixing it would require an amendment, but it should be fixed. We cannot have expired offers stored indefinitely on the ledger.