Fix GetSigOpCount and revert earlier workaround (#493)

* Fix GetSigOpCount and revert earlier workaround

* Add network argument

* Add network argument

Author: quantumagi

src/NBitcoin.Tests/sigopcount_tests.cs

@@ -1,4 +1,5 @@
 using System.Linq;
+using Moq;
 using Stratis.Bitcoin.Tests.Common;
 using Xunit;
 
@@ -39,5 +40,24 @@ public void GetSigOpCount()
             scriptSig2 = scriptSig2 + OpcodeType.OP_1 + dummy.ToBytes() + dummy.ToBytes() + s2.ToBytes();
             Assert.Equal(3U, p2sh.GetSigOpCount(KnownNetworks.Main, scriptSig2));
         }
+
+        [Fact]
+        [Trait("Core", "Core")]
+        public void GetSigOpCountForFederation()
+        {
+            PubKey[] keys = Enumerable.Range(0, 3).Select(_ => new Key(true).PubKey).ToArray();
+            var federations = new Federations();
+            federations.RegisterFederation(new Federation(keys.Take(2), 1));
+            var network = KnownNetworks.StraxRegTest;
+            network.SetPrivatePropertyValue("Federations", federations); 
+
+            // Test CScript::GetSigOpCount()
+            var s1 = new Script();
+            s1 = s1 + OpcodeType.OP_1 + OpcodeType.OP_FEDERATION + OpcodeType.OP_CHECKMULTISIG;
+            Assert.Equal(2U, s1.GetSigOpCount(true, network));
+            s1 = s1 + OpcodeType.OP_IF + OpcodeType.OP_CHECKSIG + OpcodeType.OP_ENDIF;
+            Assert.Equal(3U, s1.GetSigOpCount(true, network));
+            Assert.Equal(21U, s1.GetSigOpCount(false, network));
+        }
     }
 }

src/NBitcoin/Script.cs

@@ -854,7 +854,7 @@ public IList<Op> ToOps()
             }
         }
 
-        public uint GetSigOpCount(bool fAccurate)
+        public uint GetSigOpCount(bool fAccurate, Network network = null)
         {
             uint n = 0;
             Op lastOpcode = null;
@@ -864,7 +864,9 @@ public uint GetSigOpCount(bool fAccurate)
                     n++;
                 else if (op.Code == OpcodeType.OP_CHECKMULTISIG || op.Code == OpcodeType.OP_CHECKMULTISIGVERIFY)
                 {
-                    if (fAccurate && lastOpcode != null && lastOpcode.Code >= OpcodeType.OP_1 && lastOpcode.Code <= OpcodeType.OP_16)
+                    if (fAccurate && network?.Federations != null && lastOpcode.Code == OpcodeType.OP_FEDERATION)
+                        n += (uint)network.Federations.GetOnlyFederation().GetFederationDetails().transactionSigningKeys.Length;
+                    else if (fAccurate && lastOpcode != null && lastOpcode.Code >= OpcodeType.OP_1 && lastOpcode.Code <= OpcodeType.OP_16)
                         n += (lastOpcode.PushData == null || lastOpcode.PushData.Length == 0) ? 0U : (uint)lastOpcode.PushData[0];
                     else
                         n += 20;
@@ -913,12 +915,12 @@ public uint GetSigOpCount(Network network, Script scriptSig)
         {
             // TODO: Is the network needed?
             if (!IsPayToScriptHash(network))
-                return GetSigOpCount(true);
+                return GetSigOpCount(true, network);
             // This is a pay-to-script-hash scriptPubKey;
             // get the last item that the scriptSig
             // pushes onto the stack:
             bool validSig = new PayToScriptHashTemplate().CheckScriptSig(network, scriptSig, this);
-            return !validSig ? 0 : new Script(scriptSig.ToOps().Last().PushData).GetSigOpCount(true);
+            return !validSig ? 0 : new Script(scriptSig.ToOps().Last().PushData).GetSigOpCount(true, network);
             // ... and return its opcount:
         }
 

src/Stratis.Bitcoin.Features.Consensus/Rules/CommonRules/CoinviewRule.cs

@@ -365,7 +365,7 @@ private long CountWitnessSignatureOperation(Script scriptPubKey, WitScript witne
                 if (witParams.Program.Length == 32 && witness.PushCount > 0)
                 {
                     Script subscript = Script.FromBytesUnsafe(witness.GetUnsafePush(witness.PushCount - 1));
-                    return subscript.GetSigOpCount(true);
+                    return subscript.GetSigOpCount(true, this.Parent.Network);
                 }
             }
 

src/Stratis.Bitcoin.Features.MemoryPool/Rules/CreateMempoolEntryMempoolRule.cs

@@ -234,13 +234,10 @@ private bool AreInputsStandard(Network network, Transaction tx, MempoolCoinView
                     var redeemScript = new Script(ctx.Stack.Top(-1));
 
                     // TODO: Move this into a network-specific rule so that it only applies to Strax (the Cirrus validator already allows non-standard transactions)
-                    if (!redeemScript.ToOps().Select(o => o.Code).Contains(OpcodeType.OP_FEDERATION))
+                    if (redeemScript.GetSigOpCount(true, this.network) > MaxP2SHSigOps)
                     {
-                        if (redeemScript.GetSigOpCount(true) > MaxP2SHSigOps)
-                        {
-                            this.logger.LogTrace("(-)[SIG_OP_MAX]:false");
-                            return false;
-                        }
+                        this.logger.LogTrace("(-)[SIG_OP_MAX]:false");
+                        return false;
                     }
                 }
             }