Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/evo/assetlocktx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ using node::BlockManager;
/**
* Asset Lock Transaction
*/
bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state)
bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool is_v24_active)
{
if (tx.nType != TRANSACTION_ASSET_LOCK) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-type");
Expand Down Expand Up @@ -56,6 +56,9 @@ bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state)
if (opt_assetLockTx->getVersion() == 0 || opt_assetLockTx->getVersion() > CAssetLockPayload::CURRENT_VERSION) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-version");
}
if (!is_v24_active && opt_assetLockTx->getVersion() == CAssetLockPayload::CURRENT_VERSION) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-version-2");
}

if (opt_assetLockTx->getCreditOutputs().empty()) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-emptycreditoutputs");
Expand All @@ -68,8 +71,14 @@ bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state)
}

creditOutputsAmount += out.nValue;
if (!out.scriptPubKey.IsPayToPublicKeyHash()) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-pubKeyHash");
if (opt_assetLockTx->getVersion() >= 2) {
if (!out.scriptPubKey.IsPayToPublicKeyHash() && !out.scriptPubKey.IsPayToScriptHash()) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-script-pubkey");
}
} else {
if (!out.scriptPubKey.IsPayToPublicKeyHash()) {
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-assetlocktx-pubKeyHash");
}
}
}
if (creditOutputsAmount != returnAmount) {
Expand Down
9 changes: 5 additions & 4 deletions src/evo/assetlocktx.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ class BlockManager;
class CAssetLockPayload
{
public:
static constexpr uint8_t CURRENT_VERSION = 1;
static constexpr uint8_t INITIAL_VERSION = 1;
static constexpr uint8_t CURRENT_VERSION = 2;
static constexpr auto SPECIALTX_TYPE = TRANSACTION_ASSET_LOCK;

private:
uint8_t nVersion{CURRENT_VERSION};
std::vector<CTxOut> creditOutputs;

public:
explicit CAssetLockPayload(const std::vector<CTxOut>& creditOutputs) :
creditOutputs(creditOutputs)
explicit CAssetLockPayload(const std::vector<CTxOut>& creditOutputs, uint8_t nVersion = CURRENT_VERSION) :
nVersion(nVersion), creditOutputs(creditOutputs)
{}

CAssetLockPayload() = default;
Expand Down Expand Up @@ -154,7 +155,7 @@ class CAssetUnlockPayload
}
};

bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state);
bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool is_v24_active = false);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Require the activation flag at every call site.

Defaulting is_v24_active to false makes a missed caller silently apply pre-v24 rules instead of failing to compile. For consensus validation, I'd drop the default so new call paths must thread deployment state explicitly.

Suggested change
-bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool is_v24_active = false);
+bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool is_v24_active);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool is_v24_active = false);
bool CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool is_v24_active);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/evo/assetlocktx.h` at line 158, The declaration of CheckAssetLockTx
currently has a defaulted parameter is_v24_active = false which allows callers
to omit the activation flag; remove the default so the signature becomes bool
CheckAssetLockTx(const CTransaction& tx, TxValidationState& state, bool
is_v24_active); then update all call sites that invoke CheckAssetLockTx(...) to
pass the explicit deployment/activation boolean (thread the v24 activation state
from callers that know consensus state), ensuring compilation fails if a caller
neglects to propagate the flag.

bool CheckAssetUnlockTx(const node::BlockManager& blockman, const llmq::CQuorumManager& qman, const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev, const std::optional<CRangesSet>& indexes, TxValidationState& state);
bool GetAssetUnlockFee(const CTransaction& tx, CAmount& txfee, TxValidationState& state);

Expand Down
16 changes: 15 additions & 1 deletion src/evo/core_write.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
#include <util/std23.h>

#include <core_io.h>
#include <key_io.h>
#include <rpc/util.h>
#include <script/standard.h>
#include <util/check.h>

#include <univalue.h>
Expand Down Expand Up @@ -98,7 +100,9 @@ RPCResult CAssetLockPayload::GetJsonHelp(const std::string& key, bool optional)
{RPCResult::Type::STR, "asm", "The asm"},
{RPCResult::Type::STR_HEX, "hex", "The hex"},
{RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"},
}}}}}}
}},
{RPCResult::Type::STR, "address", /*optional=*/true, "DIP-18 Platform address (version >= 2 only)"},
}}}}
Comment on lines +103 to +105
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Nest platform address under scriptPubKey for v2 asset-lock JSON.

The new field is currently emitted as creditOutputs[].address instead of creditOutputs[].scriptPubKey.address, which can break consumers depending on the documented path.

Suggested patch
@@
                 {RPCResult::Type::OBJ, "scriptPubKey", "", {
                     {RPCResult::Type::STR, "asm", "The asm"},
                     {RPCResult::Type::STR_HEX, "hex", "The hex"},
                     {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"},
-                }},
-                {RPCResult::Type::STR, "address", /*optional=*/true, "DIP-18 Platform address (version >= 2 only)"},
+                    {RPCResult::Type::STR, "address", /*optional=*/true, "DIP-18 Platform address (version >= 2 only)"},
+                }},
         }}}}
@@
         UniValue spk(UniValue::VOBJ);
         ScriptToUniv(credit_output.scriptPubKey, spk, /*include_hex=*/true, /*include_address=*/false);
-        out.pushKV("scriptPubKey", spk);
         if (nVersion >= 2) {
             CTxDestination dest;
             if (ExtractDestination(credit_output.scriptPubKey, dest)) {
                 if (const auto* pkh = std::get_if<PKHash>(&dest)) {
-                    out.pushKV("address", EncodePlatformDestination(PlatformP2PKHDestination(uint160(*pkh))));
+                    spk.pushKV("address", EncodePlatformDestination(PlatformP2PKHDestination(uint160(*pkh))));
                 } else if (const auto* sh = std::get_if<ScriptHash>(&dest)) {
-                    out.pushKV("address", EncodePlatformDestination(PlatformP2SHDestination(uint160(*sh))));
+                    spk.pushKV("address", EncodePlatformDestination(PlatformP2SHDestination(uint160(*sh))));
                 }
             }
         }
+        out.pushKV("scriptPubKey", spk);

Also applies to: 117-128

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/evo/core_write.cpp` around lines 103 - 105, The RPC JSON currently emits
the new DIP-18 address as creditOutputs[].address; change emission so the
address is nested under creditOutputs[].scriptPubKey.address for v2 asset-locks.
Update the RPCResult/JSON building in src/evo/core_write.cpp where the
RPCResult::Type::STR "address" entry is added (both occurrences around the two
blocks noted) to instead add "scriptPubKey" as an object containing the existing
script fields plus the "address" field when version >= 2, and ensure the
produced JSON structure uses creditOutputs[].scriptPubKey.address so consumers
see the documented path.

}};
}

Expand All @@ -112,6 +116,16 @@ UniValue CAssetLockPayload::ToJson() const
UniValue spk(UniValue::VOBJ);
ScriptToUniv(credit_output.scriptPubKey, spk, /*include_hex=*/true, /*include_address=*/false);
out.pushKV("scriptPubKey", spk);
if (nVersion >= 2) {
CTxDestination dest;
if (ExtractDestination(credit_output.scriptPubKey, dest)) {
if (const auto* pkh = std::get_if<PKHash>(&dest)) {
out.pushKV("address", EncodePlatformDestination(PlatformP2PKHDestination(uint160(*pkh))));
} else if (const auto* sh = std::get_if<ScriptHash>(&dest)) {
out.pushKV("address", EncodePlatformDestination(PlatformP2SHDestination(uint160(*sh))));
}
}
}
outputs.push_back(out);
}

Expand Down
2 changes: 1 addition & 1 deletion src/evo/specialtxman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, llmq::CQuorumSn
case TRANSACTION_MNHF_SIGNAL:
return CheckMNHFTx(chainman, qman, tx, pindexPrev, state);
case TRANSACTION_ASSET_LOCK:
return CheckAssetLockTx(tx, state);
return CheckAssetLockTx(tx, state, DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_V24));
case TRANSACTION_ASSET_UNLOCK:
return CheckAssetUnlockTx(chainman.m_blockman, qman, tx, pindexPrev, indexes, state);
}
Expand Down
21 changes: 15 additions & 6 deletions src/rpc/output_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ static RPCHelpMan validateaddress()
RPCResult::Type::OBJ, "", "",
{
{RPCResult::Type::BOOL, "isvalid", "If the address is valid or not"},
{RPCResult::Type::BOOL, "isplatform", /*optional=*/true, "If the address is a DIP-18 Dash Platform address"},
{RPCResult::Type::STR, "address", /*optional=*/true, "The Dash address validated"},
{RPCResult::Type::STR_HEX, "scriptPubKey", /*optional=*/true, "The hex-encoded scriptPubKey generated by the address"},
{RPCResult::Type::BOOL, "isscript", /*optional=*/true, "If the key is a script"},
Expand All @@ -47,24 +48,32 @@ static RPCHelpMan validateaddress()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
const std::string address = request.params[0].get_str();
std::string error_msg;
CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg);
CTxDestination dest = DecodeDestination(address, error_msg);
const bool isValid = IsValidDestination(dest);
CHECK_NONFATAL(isValid == error_msg.empty());

UniValue ret(UniValue::VOBJ);
ret.pushKV("isvalid", isValid);
if (isValid) {
std::string currentAddress = EncodeDestination(dest);
ret.pushKV("address", currentAddress);
ret.pushKV("isvalid", true);
ret.pushKV("address", EncodeDestination(dest));

CScript scriptPubKey = GetScriptForDestination(dest);
ret.pushKV("scriptPubKey", HexStr(scriptPubKey));

UniValue detail = DescribeAddress(dest);
ret.pushKVs(detail);
} else {
ret.pushKV("error", error_msg);
std::string platform_error;
PlatformDestination platform_dest = DecodePlatformDestination(address, platform_error);
if (IsValidPlatformDestination(platform_dest)) {
ret.pushKV("isvalid", true);
ret.pushKV("isplatform", true);
ret.pushKV("address", EncodePlatformDestination(platform_dest));
} else {
ret.pushKV("isvalid", false);
ret.pushKV("error", error_msg);
}
}

return ret;
Expand Down
Loading
Loading