feat: Implement support for P2SH payouts for proposals (#4672)

* GOVSCRIPT deployment

* replace DEPLOYMENT_GOVSCRIPT with DEPLOYMENT_GOV_FEE

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

src/chainparams.cpp

@@ -239,7 +239,7 @@ public:
         consensus.vDeployments[Consensus::DEPLOYMENT_DIP0020].nThresholdMin = 2420; // 60% of 4032
         consensus.vDeployments[Consensus::DEPLOYMENT_DIP0020].nFalloffCoeff = 5; // this corresponds to 10 periods
 
-        // Deployment of decreased proposal fee
+        // Deployment of decreased proposal fee, script addresses for Governance Proposals
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].bit = 7;
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].nStartTime = 1638316800; // Dec 1st, 2021
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].nTimeout = 1669852800; // Dec 1st, 2022
@@ -459,7 +459,7 @@ public:
         consensus.vDeployments[Consensus::DEPLOYMENT_DIP0020].nThresholdMin = 60; // 60% of 100
         consensus.vDeployments[Consensus::DEPLOYMENT_DIP0020].nFalloffCoeff = 5; // this corresponds to 10 periods
 
-        // Deployment of decreased proposal fee
+        // Deployment of decreased proposal fee, script addresses for Governance Proposals
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].bit = 7;
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].nStartTime = 999999999999ULL; // TODO renable this before first RC
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].nTimeout = 999999999999ULL;
@@ -652,7 +652,7 @@ public:
         consensus.vDeployments[Consensus::DEPLOYMENT_DIP0020].nThresholdMin = 60; // 60% of 100
         consensus.vDeployments[Consensus::DEPLOYMENT_DIP0020].nFalloffCoeff = 5; // this corresponds to 10 periods
 
-        // Deployment of decreased proposal fee
+        // Deployment of decreased proposal fee, script addresses for Governance Proposals
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].bit = 7;
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].nStartTime = 1635724800; // Nov 1st, 2021
         consensus.vDeployments[Consensus::DEPLOYMENT_GOV_FEE].nTimeout = 999999999999ULL;

src/governance/governance.cpp

@@ -24,7 +24,7 @@ CGovernanceManager governance;
 
 int nSubmittedFinalBudget;
 
-const std::string CGovernanceManager::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-15";
+const std::string CGovernanceManager::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-16";
 const int CGovernanceManager::MAX_TIME_FUTURE_DEVIATION = 60 * 60;
 const int CGovernanceManager::RELIABLE_PROPAGATION_TIME = 60;
 
@@ -427,7 +427,9 @@ void CGovernanceManager::UpdateCachesAndClean()
         } else {
             // NOTE: triggers are handled via triggerman
             if (pObj->GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) {
-                CProposalValidator validator(pObj->GetDataAsHexString(), true);
+                bool fAllowScript = (VersionBitsTipState(Params().GetConsensus(), Consensus::DEPLOYMENT_GOV_FEE) == ThresholdState::ACTIVE);
+                bool fAllowLegacyFormat = !fAllowScript; // reusing the same bit to stop accepting proposals in legacy format
+                CProposalValidator validator(pObj->GetDataAsHexString(), fAllowLegacyFormat, fAllowScript);
                 if (!validator.Validate()) {
                     LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- set for deletion expired obj %s\n", strHash);
                     pObj->PrepareDeletion(nNow);
@@ -687,6 +689,22 @@ void CGovernanceManager::SyncObjects(CNode* pnode, CConnman& connman) const
             continue;
         }
 
+        if (pnode->nVersion < GOVSCRIPT_PROTO_VERSION && govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) {
+            // We know this proposal is valid locally, otherwise we would not store it.
+            // But we don't want to relay it to pre-GOVSCRIPT_PROTO_VERSION peers if payment_address is p2sh
+            // because they won't accept it anyway and will simply ban us eventually.
+            bool fAllowScript = (VersionBitsTipState(Params().GetConsensus(), Consensus::DEPLOYMENT_GOV_FEE) == ThresholdState::ACTIVE);
+            if (fAllowScript) {
+                CProposalValidator validator(govobj.GetDataAsHexString(), false /* no legacy format */, false /* but also no script */);
+                if (!validator.Validate(false /* ignore expiration */)) {
+                    // The only way we could get here is when proposal is valid but payment_address is actually p2sh.
+                    LogPrintf("CGovernanceManager::%s -- not syncing p2sh govobj to older node: %s, peer=%d\n", __func__,
+                        strHash, pnode->GetId());
+                    continue;
+                }
+            }
+        }
+
         // Push the inventory budget proposal message over to the other client
         LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing govobj: %s, peer=%d\n", __func__, strHash, pnode->GetId());
         pnode->PushInventory(CInv(MSG_GOVERNANCE_OBJECT, nHash));

src/governance/object.cpp

@@ -453,6 +453,8 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool fCheckCollate
 
 bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const
 {
+    AssertLockHeld(cs_main);
+
     fMissingConfirmations = false;
 
     if (fUnparsable) {
@@ -462,7 +464,9 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConf
 
     switch (nObjectType) {
     case GOVERNANCE_OBJECT_PROPOSAL: {
-        CProposalValidator validator(GetDataAsHexString(), true);
+        bool fAllowScript = (VersionBitsTipState(Params().GetConsensus(), Consensus::DEPLOYMENT_GOV_FEE) == ThresholdState::ACTIVE);
+        bool fAllowLegacyFormat = !fAllowScript; // reusing the same bit to stop accepting proposals in legacy format
+        CProposalValidator validator(GetDataAsHexString(), fAllowLegacyFormat, fAllowScript);
         // Note: It's ok to have expired proposals
         // they are going to be cleared by CGovernanceManager::UpdateCachesAndClean()
         // TODO: should they be tagged as "expired" to skip vote downloading?
@@ -673,8 +677,25 @@ void CGovernanceObject::Relay(CConnman& connman) const
         return;
     }
 
+    int minProtoVersion = MIN_GOVERNANCE_PEER_PROTO_VERSION;
+    if (nObjectType == GOVERNANCE_OBJECT_PROPOSAL) {
+        // We know this proposal is valid locally, otherwise we would not get to the point we should relay it.
+        // But we don't want to relay it to pre-GOVSCRIPT_PROTO_VERSION peers if payment_address is p2sh
+        // because they won't accept it anyway and will simply ban us eventually.
+        LOCK(cs_main);
+        bool fAllowScript = (VersionBitsTipState(Params().GetConsensus(), Consensus::DEPLOYMENT_GOV_FEE) == ThresholdState::ACTIVE);
+        if (fAllowScript) {
+            CProposalValidator validator(GetDataAsHexString(), false /* no legacy format */, false /* but also no script */);
+            if (!validator.Validate(false /* ignore expiration */)) {
+                // The only way we could get here is when proposal is valid but payment_address is actually p2sh.
+                LogPrint(BCLog::GOBJECT, "CGovernanceObject::Relay -- won't relay %s to older peers\n", GetHash().ToString());
+                minProtoVersion = GOVSCRIPT_PROTO_VERSION;
+            }
+        }
+    }
+
     CInv inv(MSG_GOVERNANCE_OBJECT, GetHash());
-    connman.RelayInv(inv, MIN_GOVERNANCE_PEER_PROTO_VERSION);
+    connman.RelayInv(inv, minProtoVersion);
 }
 
 void CGovernanceObject::UpdateSentinelVariables()

src/governance/validators.cpp

@@ -15,10 +15,11 @@
 const size_t MAX_DATA_SIZE = 512;
 const size_t MAX_NAME_SIZE = 40;
 
-CProposalValidator::CProposalValidator(const std::string& strHexData, bool fAllowLegacyFormat) :
+CProposalValidator::CProposalValidator(const std::string& strHexData, bool fAllowLegacyFormat, bool fAllowScript) :
     objJSON(UniValue::VOBJ),
     fJSONValid(false),
     fAllowLegacyFormat(fAllowLegacyFormat),
+    fAllowScript(fAllowScript),
     strErrorMessages()
 {
     if (!strHexData.empty()) {
@@ -180,7 +181,7 @@ bool CProposalValidator::ValidatePaymentAddress()
     }
 
     const CScriptID *scriptID = boost::get<CScriptID>(&dest);
-    if (scriptID) {
+    if (!fAllowScript && scriptID) {
         strErrorMessages += "script addresses are not supported;";
         return false;
     }

src/governance/validators.h

@@ -15,10 +15,11 @@ private:
     UniValue objJSON;
     bool fJSONValid;
     bool fAllowLegacyFormat;
+    bool fAllowScript;
     std::string strErrorMessages;
 
 public:
-    explicit CProposalValidator(const std::string& strDataHexIn = std::string(), bool fAllowLegacyFormat = true);
+    explicit CProposalValidator(const std::string& strDataHexIn = std::string(), bool fAllowLegacyFormat = true, bool fAllowScript = false);
 
     bool Validate(bool fCheckExpiration = true);
 

src/rpc/governance.cpp

@@ -113,7 +113,10 @@ static UniValue gobject_check(const JSONRPCRequest& request)
     CGovernanceObject govobj(hashParent, nRevision, nTime, uint256(), strDataHex);
 
     if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) {
-        CProposalValidator validator(strDataHex, false);
+        LOCK(cs_main);
+        bool fAllowScript = (VersionBitsTipState(Params().GetConsensus(), Consensus::DEPLOYMENT_GOV_FEE) == ThresholdState::ACTIVE);
+        // Note: we do not allow legacy format in RPC already, no need to reuse DEPLOYMENT_GOV_FEE
+        CProposalValidator validator(strDataHex, false, fAllowScript);
         if (!validator.Validate())  {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid proposal data, error messages:" + validator.GetErrorMessages());
         }
@@ -190,7 +193,10 @@ static UniValue gobject_prepare(const JSONRPCRequest& request)
                 govobj.GetDataAsPlainString(), govobj.GetHash().ToString());
 
     if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) {
-        CProposalValidator validator(strDataHex, false);
+        LOCK(cs_main);
+        bool fAllowScript = (VersionBitsTipState(Params().GetConsensus(), Consensus::DEPLOYMENT_GOV_FEE) == ThresholdState::ACTIVE);
+        // Note: we do not allow legacy format in RPC already, no need to reuse DEPLOYMENT_GOV_FEE
+        CProposalValidator validator(strDataHex, false, fAllowScript);
         if (!validator.Validate()) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid proposal data, error messages:" + validator.GetErrorMessages());
         }
@@ -365,7 +371,10 @@ static UniValue gobject_submit(const JSONRPCRequest& request)
                 govobj.GetDataAsPlainString(), govobj.GetHash().ToString(), txidFee.ToString());
 
     if (govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL) {
-        CProposalValidator validator(strDataHex, false);
+        LOCK(cs_main);
+        bool fAllowScript = (VersionBitsTipState(Params().GetConsensus(), Consensus::DEPLOYMENT_GOV_FEE) == ThresholdState::ACTIVE);
+        // Note: we do not allow legacy format in RPC already, no need to reuse DEPLOYMENT_GOV_FEE
+        CProposalValidator validator(strDataHex, false, fAllowScript);
         if (!validator.Validate()) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid proposal data, error messages:" + validator.GetErrorMessages());
         }

src/test/data/proposals_invalid.json

@@ -1,6 +1,7 @@
 [
 {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "yYe8KwyaUu5YswSYmB3q3ryx8XTUu9y7Ui", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
 {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": " XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
+{"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "7V11XGPxzBWxkiuw15a1Vgk7XT74tyYtCY", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
 {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc ", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
 {"end_epoch": 1491368400, "name": " dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
 {"end_epoch": 1491368400, "name": "dean miller 5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},

src/test/data/proposals_valid.json

@@ -1,7 +1,26 @@
 [
-{"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
+[
-{"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.12345678, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
+    {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
-{"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentralisé.org/dean-miller-5493"},
+    false
-{"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentralisé.org/dean-миллер-5493"},
+],
-{"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://[dead:beef:cafe:5417:affe:8FA3:deaf:feed]:/foo/"}
+[
+    {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "7V11XGPxzBWxkiuw15a1Vgk7XT74tyYtCY", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
+    true
+],
+[
+    {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.12345678, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentral.org/dean-miller-5493"},
+    false
+],
+[
+    {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentralisé.org/dean-miller-5493"},
+    false
+],
+[
+    {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://dashcentralisé.org/dean-миллер-5493"},
+    false
+],
+[
+    {"end_epoch": 1491368400, "name": "dean-miller-5493", "payment_address": "XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc", "payment_amount": 25.75, "start_epoch": 1474261086, "type": 1, "url": "http://[dead:beef:cafe:5417:affe:8FA3:deaf:feed]:/foo/"},
+    false
+]
 ]

src/test/governance_validators_tests.cpp

@@ -39,22 +39,23 @@ BOOST_AUTO_TEST_CASE(valid_proposals_test)
 
     BOOST_CHECK_MESSAGE(tests.size(), "Empty `tests`");
     for(size_t i = 0; i < tests.size(); ++i) {
-        const UniValue& objProposal = tests[i];
+        const UniValue& objProposal = tests[i][0];
+        bool fAllowScript = tests[i][1].get_bool();
 
         // legacy format
         std::string strHexData1 = CreateEncodedProposalObject(objProposal);
-        CProposalValidator validator1(strHexData1, true);
+        CProposalValidator validator1(strHexData1, true, fAllowScript);
         BOOST_CHECK_MESSAGE(validator1.Validate(false), validator1.GetErrorMessages());
         BOOST_CHECK_MESSAGE(!validator1.Validate(), validator1.GetErrorMessages());
 
         // legacy format w/validation flag off
-        CProposalValidator validator0(strHexData1, false);
+        CProposalValidator validator0(strHexData1, false, fAllowScript);
         BOOST_CHECK(!validator0.Validate());
         BOOST_CHECK_EQUAL(validator0.GetErrorMessages(), "Legacy proposal serialization format not allowed;JSON parsing error;");
 
         // new format
         std::string strHexData2 = HexStr(objProposal.write());
-        CProposalValidator validator2(strHexData2, false);
+        CProposalValidator validator2(strHexData2, false, fAllowScript);
         BOOST_CHECK_MESSAGE(validator2.Validate(false), validator2.GetErrorMessages());
         BOOST_CHECK_MESSAGE(!validator2.Validate(), validator2.GetErrorMessages());
     }
@@ -72,12 +73,12 @@ BOOST_AUTO_TEST_CASE(invalid_proposals_test)
 
         // legacy format
         std::string strHexData1 = CreateEncodedProposalObject(objProposal);
-        CProposalValidator validator1(strHexData1, true);
+        CProposalValidator validator1(strHexData1, true, false);
         BOOST_CHECK_MESSAGE(!validator1.Validate(false), validator1.GetErrorMessages());
 
         // new format
         std::string strHexData2 = HexStr(objProposal.write());
-        CProposalValidator validator2(strHexData2, false);
+        CProposalValidator validator2(strHexData2, false, false);
         BOOST_CHECK_MESSAGE(!validator2.Validate(false), validator2.GetErrorMessages());
     }
 }

src/version.h

@@ -11,7 +11,7 @@
  */
 
 
-static const int PROTOCOL_VERSION = 70220;
+static const int PROTOCOL_VERSION = 70221;
 
 //! initial proto version, to be increased after version/verack negotiation
 static const int INIT_PROTO_VERSION = 209;
@@ -51,6 +51,9 @@ static const int LLMQ_DATA_MESSAGES_VERSION = 70219;
 //! introduction of instant send deterministic lock (ISDLOCK)
 static const int ISDLOCK_PROTO_VERSION = 70220;
 
+//! GOVSCRIPT was activated in this version
+static const int GOVSCRIPT_PROTO_VERSION = 70221;
+
 // Make sure that none of the values above collide with `ADDRV2_FORMAT`.
 
 #endif // BITCOIN_VERSION_H