Skip to content

Commit

Permalink
refactor!: Governance collateral/fee is actually a commitment
Browse files Browse the repository at this point in the history
  • Loading branch information
@UdjinM6
UdjinM6 committed Sep 22, 2024
1 parent 89a7c60 commit fdf36d3
Show file tree
Hide file tree
Showing 15 changed files with 107 additions and 107 deletions.
6 changes: 3 additions & 3 deletions src/governance/common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
namespace Governance
{

Object::Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& nCollateralHash, const std::string& strDataHex) :
Object::Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& commitment_hash, const std::string& strDataHex) :
hashParent{nHashParent},
revision{nRevision},
time{nTime},
collateralHash{nCollateralHash},
m_commitment_hash{commitment_hash},
masternodeOutpoint{},
vchSig{},
vchData{ParseHex(strDataHex)}
Expand Down Expand Up @@ -46,7 +46,7 @@ UniValue Object::ToJson() const
UniValue obj(UniValue::VOBJ);
obj.pushKV("objectHash", GetHash().ToString());
obj.pushKV("parentHash", hashParent.ToString());
obj.pushKV("collateralHash", collateralHash.ToString());
obj.pushKV("commitmentHash", m_commitment_hash.ToString());
obj.pushKV("createdAt", time);
obj.pushKV("revision", revision);
UniValue data;
Expand Down
8 changes: 4 additions & 4 deletions src/governance/common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class Object
public:
Object() = default;

Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& nCollateralHash, const std::string& strDataHex);
Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& commitment_hash, const std::string& strDataHex);

UniValue ToJson() const;

Expand All @@ -55,8 +55,8 @@ class Object
/// time this object was created
int64_t time{0};

/// fee-tx
uint256 collateralHash{};
/// commitment tx id
uint256 m_commitment_hash{};

/// Masternode info for signed objects
COutPoint masternodeOutpoint;
Expand All @@ -71,7 +71,7 @@ class Object
obj.hashParent,
obj.revision,
obj.time,
obj.collateralHash,
obj.m_commitment_hash,
obj.vchData,
obj.type,
obj.masternodeOutpoint
Expand Down
6 changes: 3 additions & 3 deletions src/governance/governance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ struct sortProposalsByVotes {
bool operator()(const std::pair<CGovernanceObject*, int>& left, const std::pair<CGovernanceObject*, int>& right) const
{
if (left.second != right.second) return (left.second > right.second);
return (UintToArith256(left.first->GetCollateralHash()) > UintToArith256(right.first->GetCollateralHash()));
return (UintToArith256(left.first->GetCommitmentHash()) > UintToArith256(right.first->GetCommitmentHash()));
}
};

Expand Down Expand Up @@ -684,7 +684,7 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
// no sb_opt, no trigger
if (!sb_opt.has_value()) return std::nullopt;

//TODO: Check if nHashParentIn, nRevision and nCollateralHashIn are correct
//TODO: Check if nHashParentIn, nRevision and m_commitment_hash are correct
LOCK2(cs_main, cs);

// Check if identical trigger (equal DataHash()) is already created (signed by other masternode)
Expand Down Expand Up @@ -1177,7 +1177,7 @@ void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman)

std::string strError;
bool fMissingConfirmations;
if (govobj.IsCollateralValid(m_chainman, strError, fMissingConfirmations)) {
if (govobj.IsCommitmentValid(m_chainman, strError, fMissingConfirmations)) {
if (govobj.IsValidLocally(Assert(m_dmnman)->GetListAtChainTip(), m_chainman, strError, false)) {
AddGovernanceObject(govobj, peerman);
} else {
Expand Down
72 changes: 36 additions & 36 deletions src/governance/object.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ CGovernanceObject::CGovernanceObject()
}

CGovernanceObject::CGovernanceObject(const uint256& nHashParentIn, int nRevisionIn, int64_t nTimeIn,
const uint256& nCollateralHashIn, const std::string& strDataHexIn) :
const uint256& commitment_hash, const std::string& strDataHexIn) :
cs(),
m_obj{nHashParentIn, nRevisionIn, nTimeIn, nCollateralHashIn, strDataHexIn}
m_obj{nHashParentIn, nRevisionIn, nTimeIn, commitment_hash, strDataHexIn}
{
// PARSE JSON DATA STORAGE (VCHDATA)
LoadData();
Expand Down Expand Up @@ -375,19 +375,19 @@ UniValue CGovernanceObject::ToJson() const
void CGovernanceObject::UpdateLocalValidity(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman)
{
AssertLockHeld(cs_main);
// THIS DOES NOT CHECK COLLATERAL, THIS IS CHECKED UPON ORIGINAL ARRIVAL
// THIS DOES NOT CHECK COMMITMENT TX, THIS IS CHECKED UPON ORIGINAL ARRIVAL
fCachedLocalValidity = IsValidLocally(tip_mn_list, chainman, strLocalValidityError, false);
}


bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool fCheckCollateral) const
bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool check_commitment) const
{
bool fMissingConfirmations = false;

return IsValidLocally(tip_mn_list, chainman, strError, fMissingConfirmations, fCheckCollateral);
return IsValidLocally(tip_mn_list, chainman, strError, fMissingConfirmations, check_commitment);
}

bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const
bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations, bool check_commitment) const
{
AssertLockHeld(cs_main);

Expand All @@ -408,14 +408,14 @@ bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list,
strError = strprintf("Invalid proposal data, error messages: %s", validator.GetErrorMessages());
return false;
}
if (fCheckCollateral && !IsCollateralValid(chainman, strError, fMissingConfirmations)) {
strError = "Invalid proposal collateral";
if (check_commitment && !IsCommitmentValid(chainman, strError, fMissingConfirmations)) {
strError = "Invalid proposal commitment";
return false;
}
return true;
}
case GovernanceObject::TRIGGER: {
if (!fCheckCollateral) {
if (!check_commitment) {
// nothing else we can check here (yet?)
return true;
}
Expand All @@ -442,12 +442,12 @@ bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list,
}
}

CAmount CGovernanceObject::GetMinCollateralFee() const
CAmount CGovernanceObject::GetMinCommitmentAmount() const
{
// Only 1 type has a fee for the moment but switch statement allows for future object types
// Only 1 type has a commitment requirement for the moment but switch statement allows for future object types
switch (m_obj.type) {
case GovernanceObject::PROPOSAL: {
return GOVERNANCE_PROPOSAL_FEE_TX;
return GOVERNANCE_COMMITMENT_AMOUNT;
}
case GovernanceObject::TRIGGER: {
return 0;
Expand All @@ -458,7 +458,7 @@ CAmount CGovernanceObject::GetMinCollateralFee() const
}
}

bool CGovernanceObject::IsCollateralValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const
bool CGovernanceObject::IsCommitmentValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const
{
AssertLockHeld(cs_main);

Expand All @@ -468,22 +468,22 @@ bool CGovernanceObject::IsCollateralValid(const ChainstateManager& chainman, std

// RETRIEVE TRANSACTION IN QUESTION
uint256 nBlockHash;
CTransactionRef txCollateral = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, m_obj.collateralHash, Params().GetConsensus(), nBlockHash);
if (!txCollateral) {
strError = strprintf("Can't find collateral tx %s", m_obj.collateralHash.ToString());
LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);
CTransactionRef commitment_tx = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, m_obj.m_commitment_hash, Params().GetConsensus(), nBlockHash);
if (!commitment_tx) {
strError = strprintf("Can't find commitment tx %s", m_obj.m_commitment_hash.ToString());
LogPrintf("CGovernanceObject::%s -- %s\n", __func__, strError);
return false;
}

if (nBlockHash == uint256()) {
strError = strprintf("Collateral tx %s is not mined yet", txCollateral->ToString());
LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);
strError = strprintf("Commitment tx %s is not mined yet", commitment_tx->ToString());
LogPrintf("CGovernanceObject::%s -- %s\n", __func__, strError);
return false;
}

if (txCollateral->vout.empty()) {
if (commitment_tx->vout.empty()) {
strError = "tx vout is empty";
LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);
LogPrintf("CGovernanceObject::%s -- %s\n", __func__, strError);
return false;
}

Expand All @@ -492,28 +492,28 @@ bool CGovernanceObject::IsCollateralValid(const ChainstateManager& chainman, std
CScript findScript;
findScript << OP_RETURN << ToByteVector(nExpectedHash);

CAmount nMinFee = GetMinCollateralFee();
CAmount commitment_amount = GetMinCommitmentAmount();

LogPrint(BCLog::GOBJECT, "CGovernanceObject::IsCollateralValid -- txCollateral->vout.size() = %s, findScript = %s, nMinFee = %lld\n",
txCollateral->vout.size(), ScriptToAsmStr(findScript, false), nMinFee);
LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- commitment_tx->vout.size() = %s, findScript = %s, commitment_amount = %lld\n",
__func__, commitment_tx->vout.size(), ScriptToAsmStr(findScript, false), commitment_amount);

bool foundOpReturn = false;
for (const auto& output : txCollateral->vout) {
LogPrint(BCLog::GOBJECT, "CGovernanceObject::IsCollateralValid -- txout = %s, output.nValue = %lld, output.scriptPubKey = %s\n",
output.ToString(), output.nValue, ScriptToAsmStr(output.scriptPubKey, false));
for (const auto& output : commitment_tx->vout) {
LogPrint(BCLog::GOBJECT, "CGovernanceObject::%s -- txout = %s, output.nValue = %lld, output.scriptPubKey = %s\n",
__func__, output.ToString(), output.nValue, ScriptToAsmStr(output.scriptPubKey, false));
if (!output.scriptPubKey.IsPayToPublicKeyHash() && !output.scriptPubKey.IsUnspendable()) {
strError = strprintf("Invalid Script %s", txCollateral->ToString());
LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);
strError = strprintf("Invalid Script %s", commitment_tx->ToString());
LogPrintf("CGovernanceObject::%s -- %s\n", __func__, strError);
return false;
}
if (output.scriptPubKey == findScript && output.nValue >= nMinFee) {
if (output.scriptPubKey == findScript && output.nValue >= commitment_amount) {
foundOpReturn = true;
}
}

if (!foundOpReturn) {
strError = strprintf("Couldn't find opReturn %s in %s", nExpectedHash.ToString(), txCollateral->ToString());
LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);
strError = strprintf("Couldn't find opReturn %s in %s", nExpectedHash.ToString(), commitment_tx->ToString());
LogPrintf("CGovernanceObject::%s -- %s\n", __func__, strError);
return false;
}

Expand All @@ -528,15 +528,15 @@ bool CGovernanceObject::IsCollateralValid(const ChainstateManager& chainman, std
}
}

if (nConfirmationsIn < GOVERNANCE_FEE_CONFIRMATIONS) {
strError = strprintf("Collateral requires at least %d confirmations to be relayed throughout the network (it has only %d)", GOVERNANCE_FEE_CONFIRMATIONS, nConfirmationsIn);
if (nConfirmationsIn >= GOVERNANCE_MIN_RELAY_FEE_CONFIRMATIONS) {
if (nConfirmationsIn < GOVERNANCE_COMMITMENT_CONFIRMATIONS) {
strError = strprintf("Commitment tx requires at least %d confirmations to be relayed throughout the network (it has only %d)", GOVERNANCE_COMMITMENT_CONFIRMATIONS, nConfirmationsIn);
if (nConfirmationsIn >= GOVERNANCE_COMMITMENT_MIN_RELAY_CONFIRMATIONS) {
fMissingConfirmations = true;
strError += ", pre-accepted -- waiting for required confirmations";
} else {
strError += ", rejected -- try again later";
}
LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);
LogPrintf("CGovernanceObject::%s -- %s\n", __func__, strError);

return false;
}
Expand Down
22 changes: 11 additions & 11 deletions src/governance/object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ extern RecursiveMutex cs_main;
static constexpr double GOVERNANCE_FILTER_FP_RATE = 0.001;


static constexpr CAmount GOVERNANCE_PROPOSAL_FEE_TX = (1 * COIN);
static constexpr CAmount GOVERNANCE_COMMITMENT_AMOUNT = (1 * COIN);
static constexpr int64_t GOVERNANCE_COMMITMENT_CONFIRMATIONS = 6;
static constexpr int64_t GOVERNANCE_COMMITMENT_MIN_RELAY_CONFIRMATIONS = 1;

static constexpr int64_t GOVERNANCE_FEE_CONFIRMATIONS = 6;
static constexpr int64_t GOVERNANCE_MIN_RELAY_FEE_CONFIRMATIONS = 1;
static constexpr int64_t GOVERNANCE_UPDATE_MIN = 60 * 60;
static constexpr int64_t GOVERNANCE_DELETION_DELAY = 10 * 60;
static constexpr int64_t GOVERNANCE_ORPHAN_EXPIRATION_TIME = 10 * 60;
Expand Down Expand Up @@ -142,7 +142,7 @@ class CGovernanceObject
public:
CGovernanceObject();

CGovernanceObject(const uint256& nHashParentIn, int nRevisionIn, int64_t nTime, const uint256& nCollateralHashIn, const std::string& strDataHexIn);
CGovernanceObject(const uint256& nHashParentIn, int nRevisionIn, int64_t nTime, const uint256& commitment_hash, const std::string& strDataHexIn);

CGovernanceObject(const CGovernanceObject& other);

Expand All @@ -168,9 +168,9 @@ class CGovernanceObject
return m_obj.type;
}

const uint256& GetCollateralHash() const
const uint256& GetCommitmentHash() const
{
return m_obj.collateralHash;
return m_obj.m_commitment_hash;
}

const COutPoint& GetMasternodeOutpoint() const
Expand Down Expand Up @@ -228,12 +228,12 @@ class CGovernanceObject

// CORE OBJECT FUNCTIONS

bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool fCheckCollateral) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool check_commitment) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);

bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations, bool check_commitment) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/// Check the collateral transaction for the budget proposal/finalized budget
bool IsCollateralValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/// Check the commitment transaction for the budget proposal/finalized budget
bool IsCommitmentValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);

void UpdateLocalValidity(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman);

Expand All @@ -247,7 +247,7 @@ class CGovernanceObject
}
}

CAmount GetMinCollateralFee() const;
CAmount GetMinCommitmentAmount() const;

UniValue GetJSONObject() const;

Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class GOV
virtual ~GOV() {}
virtual void getAllNewerThan(std::vector<CGovernanceObject> &objs, int64_t nMoreThanTime) = 0;
virtual int32_t getObjAbsYesCount(const CGovernanceObject& obj, vote_signal_enum_t vote_signal) = 0;
virtual bool getObjLocalValidity(const CGovernanceObject& obj, std::string& error, bool check_collateral) = 0;
virtual bool getObjLocalValidity(const CGovernanceObject& obj, std::string& error, bool check_commitment) = 0;
virtual bool isEnabled() = 0;
virtual void setContext(NodeContext* context) {}
};
Expand Down
Loading

0 comments on commit fdf36d3

Please sign in to comment.