Check validity rules on tip-sel and check-consistency only (#1802)

* Enforce rules via flag

* make bundle validator more testable

* added test

* fix formatting

* fix mockito exception

Author: Gal Rogozinski

src/main/java/com/iota/iri/BundleValidator.java

@@ -8,6 +8,7 @@
 import com.iota.iri.utils.Converter;
 
 import java.util.*;
+import com.google.common.annotations.VisibleForTesting;
 
 /**
  * Validates bundles.
@@ -109,9 +110,11 @@ public enum Validity {
      *  we lose the last trit in the process</li>
      * </ol>
      *
-     * @param tangle          used to fetch the bundle's transactions from the persistence layer
-     * @param initialSnapshot the initial snapshot that defines the genesis for our ledger state
-     * @param tailHash        the hash of the last transaction in a bundle.
+     * @param tangle            used to fetch the bundle's transactions from the persistence layer
+     * @param enforceExtraRules true if enforce {@link #validateBundleTailApproval(Tangle, List)} and
+     *                          {@link #validateBundleTransactionsApproval(List)}
+     * @param initialSnapshot   the initial snapshot that defines the genesis for our ledger state
+     * @param tailHash          the hash of the last transaction in a bundle.
      * @return A list of transactions of the bundle contained in another list. If the bundle is valid then the tail
      * transaction's {@link TransactionViewModel#getValidity()} will return 1, else {@link
      * TransactionViewModel#getValidity()} will return -1. If the bundle is invalid then an empty list will be
@@ -121,9 +124,32 @@ public enum Validity {
      * validate it again.
      * </p>
      */
-    public List<TransactionViewModel> validate(Tangle tangle, Snapshot initialSnapshot, Hash tailHash) throws Exception {
+    public List<TransactionViewModel> validate(Tangle tangle, boolean enforceExtraRules, Snapshot initialSnapshot,
+            Hash tailHash) throws Exception {
+        int mode = getMode(enforceExtraRules);
+        return validate(tangle, initialSnapshot, tailHash, mode);
+    }
+
+    /**
+     * Does {@link #validate(Tangle, boolean, Snapshot, Hash)} but with an option of skipping some checks according to
+     * the give {@code mode}
+     *
+     * @param tangle          used to fetch the bundle's transactions from the persistence layer
+     * @param initialSnapshot the initial snapshot that defines the genesis for our ledger state
+     * @param tailHash        the hash of the last transaction in a bundle.
+     * @param mode            flags that specify which validation checks to perform
+     * @return A list of transactions of the bundle contained in another list. If the bundle is valid then the tail
+     *         transaction's {@link TransactionViewModel#getValidity()} will return 1, else
+     *         {@link TransactionViewModel#getValidity()} will return -1. If the bundle is invalid then an empty list
+     *         will be returned.
+     * @throws Exception if a persistence error occurred
+     * @implNote if {@code tailHash} was already invalidated/validated by a previous call to this method then we don't
+     *           validate it again.
+     * @see #validate(Tangle, boolean, Snapshot, Hash)
+     */
+    private List<TransactionViewModel> validate(Tangle tangle, Snapshot initialSnapshot, Hash tailHash, int mode) throws Exception {
         List<TransactionViewModel> bundleTxs = new LinkedList<>();
-        switch (validate(tangle, tailHash, MODE_VALIDATE_ALL, bundleTxs)) {
+        switch (validate(tangle, tailHash, mode, bundleTxs)) {
             case VALID:
                 if (bundleTxs.get(0).getValidity() != 1) {
                     bundleTxs.get(0).setValidity(tangle, initialSnapshot, 1);
@@ -139,7 +165,14 @@ public List<TransactionViewModel> validate(Tangle tangle, Snapshot initialSnapsh
         }
     }
 
-    private static boolean hasMode(int mode, int has) {
+    private static int getMode(boolean enforceExtraRules) {
+        if (enforceExtraRules) {
+            return MODE_VALIDATE_ALL;
+        }
+        return MODE_VALIDATE_SIGNATURES | MODE_VALIDATE_BUNDLE_HASH | MODE_VALIDATE_SEMANTICS;
+    }
+
+        private static boolean hasMode(int mode, int has) {
         return (mode & has) == has;
     }
 
@@ -160,7 +193,9 @@ private static boolean hasMode(int mode, int has) {
      * @return whether the validation criteria were passed or not
      * @throws Exception if an error occurred in the persistence layer
      */
-    public static Validity validate(Tangle tangle, Hash startTxHash, int validationMode, List<TransactionViewModel> bundleTxs) throws Exception {
+    @VisibleForTesting
+    Validity validate(Tangle tangle, Hash startTxHash, int validationMode, List<TransactionViewModel> bundleTxs)
+            throws Exception {
         TransactionViewModel startTx = TransactionViewModel.fromHash(tangle, startTxHash);
         if (startTx == null || (!hasMode(validationMode, MODE_SKIP_TAIL_TX_EXISTENCE) &&
                 (startTx.getCurrentIndex() != 0 || startTx.getValidity() == -1))) {
@@ -173,7 +208,8 @@ public static Validity validate(Tangle tangle, Hash startTxHash, int validationM
                 !hasMode(validationMode, MODE_VALIDATE_SEMANTICS));
 
         // check the semantics of the bundle: total sum, semantics per tx (current/last index), missing txs, supply
-        Validity bundleSemanticsValidity = validateBundleSemantics(startTx, bundleTxsMapping, bundleTxs, validationMode);
+        Validity bundleSemanticsValidity = validateBundleSemantics(startTx, bundleTxsMapping, bundleTxs,
+                validationMode);
         if (hasMode(validationMode, MODE_VALIDATE_SEMANTICS) && bundleSemanticsValidity != Validity.VALID) {
             return bundleSemanticsValidity;
         }
@@ -191,15 +227,19 @@ public static Validity validate(Tangle tangle, Hash startTxHash, int validationM
         }
 
         //verify that the bundle only approves tail txs
-        Validity bundleTailApprovalValidity = validateBundleTailApproval(tangle, bundleTxs);
-        if(hasMode(validationMode, MODE_VALIDATE_TAIL_APPROVAL) && bundleTailApprovalValidity != Validity.VALID){
-            return bundleTailApprovalValidity;
+        if (hasMode(validationMode, MODE_VALIDATE_TAIL_APPROVAL)) {
+            Validity bundleTailApprovalValidity = validateBundleTailApproval(tangle, bundleTxs);
+            if (bundleTailApprovalValidity != Validity.VALID) {
+                return bundleTailApprovalValidity;
+            }
         }
 
         //verify all transactions within the bundle approve via their branch the trunk transaction of the head transaction
-        Validity bundleTransactionsApprovalValidity = validateBundleTransactionsApproval(bundleTxs);
-        if(hasMode(validationMode, MODE_VALIDATE_BUNDLE_TX_APPROVAL) && bundleTransactionsApprovalValidity != Validity.VALID){
-            return bundleTransactionsApprovalValidity;
+        if (hasMode(validationMode, MODE_VALIDATE_BUNDLE_TX_APPROVAL)) {
+            Validity bundleTransactionsApprovalValidity = validateBundleTransactionsApproval(bundleTxs);
+            if (bundleTransactionsApprovalValidity != Validity.VALID) {
+                return bundleTransactionsApprovalValidity;
+            }
         }
 
         // verify the signatures of input transactions
@@ -222,7 +262,8 @@ public static Validity validate(Tangle tangle, Hash startTxHash, int validationM
      * @param bundleTxs        an empty list which gets filled with the transactions in order of trunk ordering
      * @return whether the bundle is semantically valid
      */
-    public static Validity validateBundleSemantics(TransactionViewModel startTx, Map<Hash, TransactionViewModel> bundleTxsMapping, List<TransactionViewModel> bundleTxs) {
+    public Validity validateBundleSemantics(TransactionViewModel startTx,
+            Map<Hash, TransactionViewModel> bundleTxsMapping, List<TransactionViewModel> bundleTxs) {
         return validateBundleSemantics(startTx, bundleTxsMapping, bundleTxs, MODE_VALIDATE_SEMANTICS);
     }
 
@@ -245,7 +286,9 @@ public static Validity validateBundleSemantics(TransactionViewModel startTx, Map
      * @param validationMode   the used validation mode
      * @return whether the bundle is semantically valid
      */
-    private static Validity validateBundleSemantics(TransactionViewModel startTx, Map<Hash, TransactionViewModel> bundleTxsMapping, List<TransactionViewModel> bundleTxs, int validationMode) {
+    private Validity validateBundleSemantics(TransactionViewModel startTx,
+            Map<Hash, TransactionViewModel> bundleTxsMapping, List<TransactionViewModel> bundleTxs,
+            int validationMode) {
         TransactionViewModel tvm = startTx;
         final long lastIndex = tvm.lastIndex();
         long bundleValue = 0;
@@ -410,7 +453,8 @@ public static boolean isInconsistent(Collection<TransactionViewModel> transactio
      * @param bundleTxs list of transactions that are in a bundle.
      * @return Whether the bundle tx chain is valid.
      */
-    public static Validity validateBundleTransactionsApproval(List<TransactionViewModel> bundleTxs){
+    @VisibleForTesting
+    Validity validateBundleTransactionsApproval(List<TransactionViewModel> bundleTxs) {
         Hash headTrunkTransactionHash = bundleTxs.get(bundleTxs.size() - 1).getTrunkTransactionHash();
         for(int i = 0; i < bundleTxs.size() - 1; i++){
             if(!bundleTxs.get(i).getBranchTransactionHash().equals(headTrunkTransactionHash)){
@@ -426,7 +470,8 @@ public static Validity validateBundleTransactionsApproval(List<TransactionViewMo
      * @param bundleTxs The txs in the bundle.
      * @return Whether the bundle approves only tails.
      */
-    public static Validity validateBundleTailApproval(Tangle tangle, List<TransactionViewModel> bundleTxs) throws Exception {
+    @VisibleForTesting
+    Validity validateBundleTailApproval(Tangle tangle, List<TransactionViewModel> bundleTxs) throws Exception {
         TransactionViewModel headTx = bundleTxs.get(bundleTxs.size() - 1);
         TransactionViewModel bundleTrunkTvm = headTx.getTrunkTransaction(tangle);
         TransactionViewModel bundleBranchTvm = headTx.getBranchTransaction(tangle);

src/main/java/com/iota/iri/service/API.java

@@ -389,10 +389,14 @@ private AbstractResponse checkConsistencyStatement(List<String> transactionsList
                 state = false;
                 info = "tails are not solid (missing a referenced tx): " + transaction;
                 break;
-            } else if (bundleValidator.validate(tangle, snapshotProvider.getInitialSnapshot(), txVM.getHash()).isEmpty()) {
-                state = false;
-                info = "tails are not consistent (bundle is invalid): " + transaction;
-                break;
+            } else {
+                if (bundleValidator
+                        .validate(tangle, true, snapshotProvider.getInitialSnapshot(), txVM.getHash())
+                        .isEmpty()) {
+                    state = false;
+                    info = "tails are not consistent (bundle is invalid): " + transaction;
+                    break;
+                }
             }
         }
 

src/main/java/com/iota/iri/service/ledger/LedgerService.java

@@ -2,6 +2,7 @@
 
 import com.iota.iri.controllers.MilestoneViewModel;
 import com.iota.iri.model.Hash;
+import com.iota.iri.storage.Tangle;
 
 import java.util.List;
 import java.util.Map;
@@ -99,12 +100,16 @@ public interface LedgerService {
      * </p>
      *
      * @param visitedTransactions a set of transaction hashes that shall be considered to be visited already
-     * @param startTransaction the transaction that marks the start of the dag traversal and that has its approvees
-     *                        examined
+     * @param startTransaction    the transaction that marks the start of the dag traversal and that has its approvees
+     *                            examined
+     * @param enforceExtraRules   enforce {@link com.iota.iri.BundleValidator#validateBundleTransactionsApproval(List)}
+     *                            and {@link com.iota.iri.BundleValidator#validateBundleTailApproval(Tangle, List)}.
+     *                            Enforcing them may break backwards compatibility.
      * @return a map of the balance changes (addresses associated to their balance) or {@code null} if the balance could
      *         not be generated due to inconsistencies
      * @throws LedgerException if anything unexpected happens while generating the balance changes
      */
-    Map<Hash, Long> generateBalanceDiff(Set<Hash> visitedTransactions, Hash startTransaction, int milestoneIndex)
+    Map<Hash, Long> generateBalanceDiff(Set<Hash> visitedTransactions, Hash startTransaction, int milestoneIndex,
+            boolean enforceExtraRules)
             throws LedgerException;
 }

src/main/java/com/iota/iri/service/ledger/impl/LedgerServiceImpl.java

@@ -133,7 +133,7 @@ public boolean isBalanceDiffConsistent(Set<Hash> approvedHashes, Map<Hash, Long>
         }
         Set<Hash> visitedHashes = new HashSet<>(approvedHashes);
         Map<Hash, Long> currentState = generateBalanceDiff(visitedHashes, tip,
-                snapshotProvider.getLatestSnapshot().getIndex());
+                snapshotProvider.getLatestSnapshot().getIndex(), true);
         if (currentState == null) {
             return false;
         }
@@ -152,7 +152,8 @@ public boolean isBalanceDiffConsistent(Set<Hash> approvedHashes, Map<Hash, Long>
     }
 
     @Override
-    public Map<Hash, Long> generateBalanceDiff(Set<Hash> visitedTransactions, Hash startTransaction, int milestoneIndex)
+    public Map<Hash, Long> generateBalanceDiff(Set<Hash> visitedTransactions, Hash startTransaction, int milestoneIndex,
+            boolean enforceExtraRules)
             throws LedgerException {
 
         Map<Hash, Long> state = new HashMap<>();
@@ -176,7 +177,8 @@ public Map<Hash, Long> generateBalanceDiff(Set<Hash> visitedTransactions, Hash s
                     if (!milestoneService.isTransactionConfirmed(transactionViewModel, milestoneIndex)) {
 
                         final List<TransactionViewModel> bundleTransactions = bundleValidator.validate(tangle,
-                                snapshotProvider.getInitialSnapshot(), transactionViewModel.getHash());
+                                enforceExtraRules, snapshotProvider.getInitialSnapshot(),
+                                transactionViewModel.getHash());
 
                         if (bundleTransactions.isEmpty()) {
                             return null;
@@ -257,7 +259,7 @@ private boolean generateStateDiff(MilestoneViewModel milestone) throws LedgerExc
                 try {
                     Hash tail = transactionViewModel.getHash();
                     Map<Hash, Long> balanceChanges = generateBalanceDiff(new HashSet<>(), tail,
-                            snapshotProvider.getLatestSnapshot().getIndex());
+                            snapshotProvider.getLatestSnapshot().getIndex(), false);
                     successfullyProcessed = balanceChanges != null;
                     if (successfullyProcessed) {
                         successfullyProcessed = snapshotProvider.getLatestSnapshot().patchedState(

src/main/java/com/iota/iri/service/milestone/impl/MilestoneServiceImpl.java

@@ -162,7 +162,7 @@ public MilestoneValidity validateMilestone(TransactionViewModel transactionViewM
                 return existingMilestone.getHash().equals(transactionViewModel.getHash()) ? VALID : INVALID;
             }
 
-            final List<TransactionViewModel> bundleTransactions = bundleValidator.validate(tangle,
+            final List<TransactionViewModel> bundleTransactions = bundleValidator.validate(tangle, true,
                     snapshotProvider.getInitialSnapshot(), transactionViewModel.getHash());
 
             if (bundleTransactions.isEmpty()) {

src/main/java/com/iota/iri/service/spentaddresses/impl/SpentAddressesServiceImpl.java

@@ -122,7 +122,7 @@ private boolean wasTransactionSpentFrom(TransactionViewModel tx) throws Exceptio
 
     private boolean isBundleValid(Hash tailHash) throws Exception {
         List<TransactionViewModel> validation =
-                bundleValidator.validate(tangle, snapshotProvider.getInitialSnapshot(), tailHash);
+                bundleValidator.validate(tangle, false, snapshotProvider.getInitialSnapshot(), tailHash);
         return (CollectionUtils.isNotEmpty(validation) && validation.get(0).getValidity() == 1);
     }