MerosCrypto / Meros

An instant and feeless cryptocurrency for the future, secured by the Merit Caching Consensus Mechanism.
https://meroscrypto.io
Other
82 stars 19 forks source link

Check to see if a TX was already mentioned on chain uses getTransaction; it needs to use getStatus. #234

Closed kayabaNerve closed 4 years ago

kayabaNerve commented 4 years ago

getTransaction solely says we have the Transaction, not any Verifications.

Discovered while working on #233. While it is related, that shouldn't be the cause. getBlockTemplate explicitly doesn't include TXs which don't have mentioned/mentionable parents, which is confirmed by a e2e test. If they weren't in RAM, they would have been synced. If they were, the VP would still be applied.

kayabaNerve commented 4 years ago

This is incorrect. We do have the following block, which is proper:

  #Verify the included packets.
  for packet in result[0].body.packets:
    #Verify the predecessors of every Transaction are already mentioned on the chain OR also in this Block.
    var tx: Transaction
    try:
      tx = manager.functions.transactions.getTransaction(packet.hash)
    except IndexError as e:
      panic("Couldn't get a Transaction we're confirmed to have: " & e.msg)

    if not ((tx of Claim) or ((tx of Data) and cast[Data](tx).isFirstData)):
      for input in tx.inputs:
        if not (manager.functions.consensus.hasArchivedPacket(input.hash) or includedTXs.contains(input.hash)):
          raise newLoggedException(ValueError, "Block's Transactions have predecessors which have yet to be mentioned on chain.")

My confusion was from the following block's error message:

        #Make sure we have already added every input.
        if not first:
          for input in tx.inputs:
            try:
              discard manager.functions.transactions.getTransaction(input.hash)
            #This TX is missing an input.
            except IndexError:
              #Look for the input in the pending Transactions.
              if transactions.hasKey(input.hash):
                #If it's there, add this Transaction to be handled later.
                todo[tx.hash] = tx
                break thisTX
              else:
                raise newLoggedException(ValueError, "Block includes Verifications of a Transaction which has not had all its inputs mentioned in previous blocks/this block.")

If this was the only check, it would count Transactions in our mempool as mentioned. That said, this error is accurate; it just requires the other block to be comprehensive.