Merge branch 'release-v1.8.5'

Gal Rogozinski · Gal Rogozinski · commit af9b4c50eb2b · 2020-03-23T13:54:52.000+02:00
diff --git a/changelog.txt b/changelog.txt
@@ -1,3 +1,10 @@
+1.8.5
+
+Protocol-Change:
+    Added bundle validity rules - check that bundles confirm only tails and 2 bundles at most (#1786)
+    Check validity rules on tip-sel and check-consistency only (#1802)
+
+
 1.8.4
 
 Hotfix: Ensure proper creation of solid entrypoints (#1702)
diff --git a/pom.xml b/pom.xml
@@ -5,7 +5,7 @@
 
     <groupId>com.iota</groupId>
     <artifactId>iri</artifactId>
-    <version>1.8.4</version>
+    <version>1.8.5</version>
     <name>IRI</name>
     <description>IOTA Reference Implementation</description>
 
diff --git a/python-regression/tests/features/steps/api_test_steps.py b/python-regression/tests/features/steps/api_test_steps.py
@@ -120,11 +120,11 @@ def compare_thread_return(step, api_call):
             del response_list['duration']
         if 'info' in response_list:
             del response_list['info']
-        response_keys = response_list.keys()
+        response_keys = list(response_list.keys())
 
         expected_values = {}
         api_utils.prepare_options(step.hashes,expected_values)
-        keys = expected_values.keys()
+        keys = list(expected_values.keys())
 
         # Confirm that the lists are of equal length before comparing
         assert len(keys) == len(response_keys), \
diff --git a/python-regression/util/neighbor_logic/neighbor_logic.py b/python-regression/util/neighbor_logic/neighbor_logic.py
@@ -26,5 +26,5 @@ def check_if_neighbors(api, neighbors, expected_neighbor):
 
     if is_neighbor is False:
         tcp_address = "tcp://" + expected_neighbor
-        api.add_neighbors([tcp_address.decode()])
+        api.add_neighbors([tcp_address])
         logger.info('{} added as neighbor'.format(tcp_address))
diff --git a/python-regression/util/test_logic/value_fetch_logic.py b/python-regression/util/test_logic/value_fetch_logic.py
@@ -76,7 +76,7 @@ def fetch_node_address(value):
     host = world.machine['nodes'][value]['host']
     port = world.machine['nodes'][value]['ports']['gossip-tcp']
     address = "tcp://" + host + ":" + str(port)
-    return [address.decode()]
+    return [address]
 
 
 def fetch_static_value(value):
diff --git a/src/main/java/com/iota/iri/BundleValidator.java b/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.
@@ -55,22 +56,33 @@ public enum Validity {
      */
     public static final int MODE_VALIDATE_SEMANTICS = 1 << 2;
 
+    /**
+     * Instructs the validation code to validate all transactions within the bundle approve via their branch the trunk
+     * transaction of the head transaction
+     */
+    public static final int MODE_VALIDATE_BUNDLE_TX_APPROVAL = 1 << 3;
+
+    /**
+     * Instructs the validation code to validate that the bundle only approves tail txs.
+     */
+    public static final int MODE_VALIDATE_TAIL_APPROVAL = 1 << 4;
+
     /**
      * Instructs the validation code to fully validate the semantics, bundle hash and signatures of the given bundle.
      */
-    public static final int MODE_VALIDATE_ALL = MODE_VALIDATE_SIGNATURES | MODE_VALIDATE_BUNDLE_HASH | MODE_VALIDATE_SEMANTICS;
+    public static final int MODE_VALIDATE_ALL = MODE_VALIDATE_SIGNATURES | MODE_VALIDATE_BUNDLE_HASH | MODE_VALIDATE_SEMANTICS | MODE_VALIDATE_TAIL_APPROVAL | MODE_VALIDATE_BUNDLE_TX_APPROVAL;
 
     /**
      * Instructs the validation code to skip checking the bundle's already computed validity and instead to proceed to
      * validate the bundle further.
      */
-    public static final int MODE_SKIP_CACHED_VALIDITY = 1 << 3;
+    public static final int MODE_SKIP_CACHED_VALIDITY = 1 << 5;
 
     /**
      * Instructs the validation code to skip checking whether the tail transaction is present or a tail transaction was
      * given as the start transaction.
      */
-    public static final int MODE_SKIP_TAIL_TX_EXISTENCE = 1 << 4;
+    public static final int MODE_SKIP_TAIL_TX_EXISTENCE = 1 << 6;
 
     /**
      * Fetches a bundle of transactions identified by the {@code tailHash} and validates the transactions. Bundle is a
@@ -85,6 +97,8 @@ public enum Validity {
      *  <li>Total bundle value is 0 (inputs and outputs are balanced)</li>
      *  <li>Recalculate the bundle hash by absorbing and squeezing the transactions' essence</li>
      *  <li>Validate the signature on input transactions</li>
+     *  <li>The bundle must only approve tail transactions</li>
+     *  <li>All transactions within the bundle approve via their branch the trunk transaction of the head transaction.</li>
      * </ol>
      * <p>
      * As well as the following syntactic checks:
@@ -96,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
@@ -108,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);
@@ -126,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;
     }
 
@@ -147,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))) {
@@ -160,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;
         }
@@ -177,6 +226,21 @@ public static Validity validate(Tangle tangle, Hash startTxHash, int validationM
             return bundleHashValidity;
         }
 
+        //verify that the bundle only approves tail txs
+        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
+        if (hasMode(validationMode, MODE_VALIDATE_BUNDLE_TX_APPROVAL)) {
+            Validity bundleTransactionsApprovalValidity = validateBundleTransactionsApproval(bundleTxs);
+            if (bundleTransactionsApprovalValidity != Validity.VALID) {
+                return bundleTransactionsApprovalValidity;
+            }
+        }
 
         // verify the signatures of input transactions
         if (hasMode(validationMode, MODE_VALIDATE_SIGNATURES)) {
@@ -198,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);
     }
 
@@ -221,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;
@@ -379,6 +446,39 @@ public static boolean isInconsistent(Collection<TransactionViewModel> transactio
         return (sum != 0 || transactionViewModels.isEmpty());
     }
 
+    /**
+     * A bundle is invalid if The branch transaction hash of the non head transactions within a bundle, is not the same
+     * as the trunk transaction hash of the head transaction.
+     *
+     * @param bundleTxs list of transactions that are in a bundle.
+     * @return Whether the bundle tx chain is valid.
+     */
+    @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)){
+                return Validity.INVALID;
+            }
+        }
+        return Validity.VALID;
+    }
+
+    /**
+     * A bundle is invalid if the trunk and branch transactions approved by the bundle are non tails.
+     *
+     * @param bundleTxs The txs in the bundle.
+     * @return Whether the bundle approves only tails.
+     */
+    @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);
+        return bundleTrunkTvm != null && bundleBranchTvm != null && bundleBranchTvm.getCurrentIndex() == 0
+                && bundleTrunkTvm.getCurrentIndex() == 0 ? Validity.VALID : Validity.INVALID;
+    }
+
     /**
      * Traverses down the given {@code tail} trunk until all transactions that belong to the same bundle (identified by
      * the bundle hash) are found and loaded.
diff --git a/src/main/java/com/iota/iri/network/NeighborRouterImpl.java b/src/main/java/com/iota/iri/network/NeighborRouterImpl.java
@@ -508,7 +508,7 @@ private boolean finalizeHandshake(String identity, Neighbor neighbor, SocketChan
                 return false;
             case FAILED:
                 // faulty handshaking
-                log.error("dropping connection to neighbor {} as handshaking was faulty", identity);
+                log.warn("dropping connection to neighbor {} as handshaking was faulty", identity);
                 closeNeighborConnection(channel, identity, selector);
                 return false;
             default:
diff --git a/src/main/java/com/iota/iri/service/API.java b/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;
+                }
             }
         }
 
diff --git a/src/main/java/com/iota/iri/service/ledger/LedgerService.java b/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;
 }
diff --git a/src/main/java/com/iota/iri/service/ledger/impl/LedgerServiceImpl.java b/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(
diff --git a/src/main/java/com/iota/iri/service/milestone/impl/MilestoneServiceImpl.java b/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()) {
diff --git a/src/main/java/com/iota/iri/service/spentaddresses/impl/SpentAddressesServiceImpl.java b/src/main/java/com/iota/iri/service/spentaddresses/impl/SpentAddressesServiceImpl.java
diff --git a/src/test/java/com/iota/iri/BundleValidatorTest.java b/src/test/java/com/iota/iri/BundleValidatorTest.java
diff --git a/src/test/java/com/iota/iri/TangleMockUtils.java b/src/test/java/com/iota/iri/TangleMockUtils.java
diff --git a/src/test/java/com/iota/iri/TransactionTestUtils.java b/src/test/java/com/iota/iri/TransactionTestUtils.java
diff --git a/src/test/java/com/iota/iri/service/ledger/impl/LedgerServiceImplTest.java b/src/test/java/com/iota/iri/service/ledger/impl/LedgerServiceImplTest.java
