Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spark coinbase #1494

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+534 −85
Open

Spark coinbase #1494

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
90c9b9c
First step to implement spark coinbase
levonpetrosyan93 Jul 1, 2024
63f11f1
Masternode Spark payouts implemented and fixes
levonpetrosyan93 Aug 26, 2024
0ae0b80
More fixes and cleanup
levonpetrosyan93 Sep 23, 2024
d9a3276
Merge branch 'master' into spark_coinbase
levonpetrosyan93 Sep 23, 2024
dc29f5b
Minor fixes
levonpetrosyan93 Oct 7, 2024
05d5c84
Make spark coinbase imature untill 100blocks passed
levonpetrosyan93 Nov 11, 2024
484e80b
Bring back accidentally removed line
levonpetrosyan93 Nov 19, 2024
2738e20
Review comments applied, and more
levonpetrosyan93 Nov 25, 2024
98d5fc4
Small cleanup
levonpetrosyan93 Nov 25, 2024
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
14 changes: 12 additions & 2 deletions src/chain.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ class CBlockIndex
//! Map id to <hash of the set>
std::map<int, std::vector<unsigned char>> anonymitySetHash;
//! Map id to spark coin
std::map<int, std::vector<spark::Coin>> sparkMintedCoins;
std::map<int, std::vector<std::pair<spark::Coin, bool>>> sparkMintedCoins;
//! Map id to <hash of the set>
std::map<int, std::vector<unsigned char>> sparkSetHash;
//! map spark coin S to tx hash, this is used when you run with -mobile
Expand Down Expand Up @@ -560,7 +560,17 @@ class CDiskBlockIndex : public CBlockIndex

if (!(s.GetType() & SER_GETHASH)
&& nHeight >= params.nSparkStartBlock) {
READWRITE(sparkMintedCoins);
if (nHeight >=params.nSparkCoinbase) {
READWRITE(sparkMintedCoins);
} else {
std::map<int, std::vector<spark::Coin>> sparkCoins;
READWRITE(sparkCoins);
for (auto& itr : sparkCoins) {
Copy link

Choose a reason for hiding this comment

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

When serializing (writing), won't this write an empty map and then iterate over that empty map? Is that really the intent?

sparkMintedCoins[itr.first].reserve(itr.second.size());
for (auto& mint : itr.second)
sparkMintedCoins[itr.first].emplace_back(std::make_pair(mint, false));
}
}
READWRITE(sparkSetHash);
READWRITE(spentLTags);

Expand Down
4 changes: 4 additions & 0 deletions src/chainparams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ class CMainParams : public CChainParams {
consensus.nLelantusStartBlock = ZC_LELANTUS_STARTING_BLOCK;
consensus.nLelantusFixesStartBlock = ZC_LELANTUS_FIXES_START_BLOCK;
consensus.nSparkStartBlock = SPARK_START_BLOCK;
consensus.nSparkCoinbase = SPARK_START_BLOCK; //TODO levon
consensus.nLelantusGracefulPeriod = LELANTUS_GRACEFUL_PERIOD;
consensus.nZerocoinV2MintMempoolGracefulPeriod = ZC_V2_MINT_GRACEFUL_MEMPOOL_PERIOD;
consensus.nZerocoinV2MintGracefulPeriod = ZC_V2_MINT_GRACEFUL_PERIOD;
Expand Down Expand Up @@ -721,6 +722,7 @@ class CTestNetParams : public CChainParams {
consensus.nLelantusFixesStartBlock = ZC_LELANTUS_TESTNET_FIXES_START_BLOCK;

consensus.nSparkStartBlock = SPARK_TESTNET_START_BLOCK;
consensus.nSparkCoinbase = SPARK_TESTNET_START_BLOCK; //TODO levon put the real HW block
consensus.nLelantusGracefulPeriod = LELANTUS_TESTNET_GRACEFUL_PERIOD;

consensus.nZerocoinV2MintMempoolGracefulPeriod = ZC_V2_MINT_TESTNET_GRACEFUL_MEMPOOL_PERIOD;
Expand Down Expand Up @@ -980,6 +982,7 @@ class CDevNetParams : public CChainParams {
consensus.nLelantusFixesStartBlock = 1;

consensus.nSparkStartBlock = 1500;
consensus.nSparkCoinbase = 1500; //TODO levon put the real HW block
consensus.nLelantusGracefulPeriod = 6000;

consensus.nMaxSigmaInputPerBlock = ZC_SIGMA_INPUT_LIMIT_PER_BLOCK;
Expand Down Expand Up @@ -1219,6 +1222,7 @@ class CRegTestParams : public CChainParams {
consensus.nLelantusStartBlock = 400;
consensus.nLelantusFixesStartBlock = 400;
consensus.nSparkStartBlock = 1000;
consensus.nSparkCoinbase = consensus.nSparkStartBlock;
consensus.nExchangeAddressStartBlock = 1000;
consensus.nLelantusGracefulPeriod = 1500;
consensus.nZerocoinV2MintMempoolGracefulPeriod = 1;
Expand Down
2 changes: 2 additions & 0 deletions src/consensus/params.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ struct Params {

int nSparkStartBlock;

int nSparkCoinbase;

Comment on lines +267 to +268
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Let me analyze the results and generate a final response.

TODO comments and inconsistent initialization need attention

The verification reveals several concerns:

  • There are TODO comments indicating unfinished work for nSparkCoinbase values
  • Mainnet and testnet use placeholder values with TODO comments
  • Regtest uses a hardcoded value of 1500
  • DevNet sets nSparkCoinbase equal to nSparkStartBlock

Required actions:

  • Replace TODO comments with proper implementation in mainnet and testnet
  • Review the hardcoded value 1500 in regtest
  • Consider standardizing the relationship between nSparkCoinbase and nSparkStartBlock across all networks (currently only DevNet follows a clear pattern)
🔗 Analysis chain

Verify initialization across different networks.

Ensure that nSparkCoinbase is properly initialized in chainparams.cpp for all networks (mainnet, testnet, regtest) and that the values are coordinated with nSparkStartBlock.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check initialization of nSparkCoinbase across network params
# and its relationship with nSparkStartBlock

# Search for nSparkCoinbase initialization
echo "Checking nSparkCoinbase initialization:"
rg "nSparkCoinbase =" -A 1

# Search for nSparkStartBlock initialization for comparison
echo -e "\nChecking nSparkStartBlock initialization:"
rg "nSparkStartBlock =" -A 1

# Verify both parameters are set in all network params
echo -e "\nVerifying presence in network params:"
rg "class C(Main|TestNet|RegTest|DevNet)Params"

Length of output: 2045

int nLelantusGracefulPeriod;

// Lelantus Blacklist
Expand Down
18 changes: 18 additions & 0 deletions src/evo/deterministicmns.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,18 @@ std::string CDeterministicMNState::ToString() const
std::string operatorPayoutAddress = "none";
if (ExtractDestination(scriptPayout, dest)) {
payoutAddress = CBitcoinAddress(dest).ToString();
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptPayout);
if (!strScriptPayout.empty())
payoutAddress = strScriptPayout;
Copy link

Choose a reason for hiding this comment

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

std::move(), also in similar cases below

}

if (ExtractDestination(scriptOperatorPayout, dest)) {
operatorPayoutAddress = CBitcoinAddress(dest).ToString();
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptOperatorPayout);
if (!strScriptPayout.empty())
operatorPayoutAddress = strScriptPayout;
}

return strprintf("CDeterministicMNState(nRegisteredHeight=%d, nLastPaidHeight=%d, nPoSePenalty=%d, nPoSeRevivedHeight=%d, nPoSeBanHeight=%d, nRevocationReason=%d, "
Expand All @@ -59,11 +68,20 @@ void CDeterministicMNState::ToJson(UniValue& obj) const
if (ExtractDestination(scriptPayout, dest)) {
CBitcoinAddress payoutAddress(dest);
obj.push_back(Pair("payoutAddress", payoutAddress.ToString()));
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptPayout);
if (!strScriptPayout.empty())
obj.push_back(Pair("payoutAddress", strScriptPayout));
}

obj.push_back(Pair("pubKeyOperator", pubKeyOperator.Get().ToString()));
if (ExtractDestination(scriptOperatorPayout, dest)) {
CBitcoinAddress operatorPayoutAddress(dest);
obj.push_back(Pair("operatorPayoutAddress", operatorPayoutAddress.ToString()));
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptOperatorPayout);
if (!strScriptPayout.empty())
obj.push_back(Pair("operatorPayoutAddress", strScriptPayout));
}
}

Expand Down
36 changes: 31 additions & 5 deletions src/evo/providertx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,12 @@ bool CheckProRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValid
if (ptx.keyIDOwner.IsNull() || !ptx.pubKeyOperator.IsValid() || ptx.keyIDVoting.IsNull()) {
return state.DoS(10, false, REJECT_INVALID, "bad-protx-key-null");
}
if (!ptx.scriptPayout.IsPayToPublicKeyHash() && !ptx.scriptPayout.IsPayToScriptHash()) {
if (!ptx.scriptPayout.IsPayToPublicKeyHash() && !ptx.scriptPayout.IsPayToScriptHash() && !spark::IsPayToSparkAddress(ptx.scriptPayout)) {
return state.DoS(10, false, REJECT_INVALID, "bad-protx-payee");
}

CTxDestination payoutDest;
if (!ExtractDestination(ptx.scriptPayout, payoutDest)) {
if (!ExtractDestination(ptx.scriptPayout, payoutDest) && !spark::IsPayToSparkAddress(ptx.scriptPayout)) {
// should not happen as we checked script types before
return state.DoS(10, false, REJECT_INVALID, "bad-protx-payee-dest");
}
Expand Down Expand Up @@ -248,7 +248,7 @@ bool CheckProUpServTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVa
// don't allow to set operator reward payee in case no operatorReward was set
return state.DoS(10, false, REJECT_INVALID, "bad-protx-operator-payee");
}
if (!ptx.scriptOperatorPayout.IsPayToPublicKeyHash() && !ptx.scriptOperatorPayout.IsPayToScriptHash()) {
if (!ptx.scriptOperatorPayout.IsPayToPublicKeyHash() && !ptx.scriptOperatorPayout.IsPayToScriptHash() && !spark::IsPayToSparkAddress(ptx.scriptOperatorPayout)) {
return state.DoS(10, false, REJECT_INVALID, "bad-protx-operator-payee");
}
}
Expand Down Expand Up @@ -286,12 +286,12 @@ bool CheckProUpRegTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVal
if (!ptx.pubKeyOperator.IsValid() || ptx.keyIDVoting.IsNull()) {
return state.DoS(10, false, REJECT_INVALID, "bad-protx-key-null");
}
if (!ptx.scriptPayout.IsPayToPublicKeyHash() && !ptx.scriptPayout.IsPayToScriptHash()) {
if (!ptx.scriptPayout.IsPayToPublicKeyHash() && !ptx.scriptPayout.IsPayToScriptHash() && !spark::IsPayToSparkAddress(ptx.scriptPayout)) {
return state.DoS(10, false, REJECT_INVALID, "bad-protx-payee");
}

CTxDestination payoutDest;
if (!ExtractDestination(ptx.scriptPayout, payoutDest)) {
if (!ExtractDestination(ptx.scriptPayout, payoutDest) && !spark::IsPayToSparkAddress(ptx.scriptPayout)) {
// should not happen as we checked script types before
return state.DoS(10, false, REJECT_INVALID, "bad-protx-payee-dest");
}
Expand Down Expand Up @@ -415,6 +415,10 @@ std::string CProRegTx::ToString() const
std::string payee = "unknown";
if (ExtractDestination(scriptPayout, dest)) {
payee = CBitcoinAddress(dest).ToString();
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptPayout);
if (!strScriptPayout.empty())
payee = strScriptPayout;
}

return strprintf("CProRegTx(nVersion=%d, collateralOutpoint=%s, addr=%s, nOperatorReward=%f, ownerAddress=%s, pubKeyOperator=%s, votingAddress=%s, scriptPayout=%s)",
Expand All @@ -436,7 +440,13 @@ void CProRegTx::ToJson(UniValue& obj) const
if (ExtractDestination(scriptPayout, dest)) {
CBitcoinAddress bitcoinAddress(dest);
obj.push_back(Pair("payoutAddress", bitcoinAddress.ToString()));
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptPayout);
if (!strScriptPayout.empty())
obj.push_back(Pair("payoutAddress", strScriptPayout));
}


obj.push_back(Pair("pubKeyOperator", pubKeyOperator.ToString()));
obj.push_back(Pair("operatorReward", (double)nOperatorReward / 100));

Expand All @@ -449,6 +459,10 @@ std::string CProUpServTx::ToString() const
std::string payee = "unknown";
if (ExtractDestination(scriptOperatorPayout, dest)) {
payee = CBitcoinAddress(dest).ToString();
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptOperatorPayout);
if (!strScriptPayout.empty())
payee = strScriptPayout;
}

return strprintf("CProUpServTx(nVersion=%d, proTxHash=%s, addr=%s, operatorPayoutAddress=%s)",
Expand All @@ -466,6 +480,10 @@ void CProUpServTx::ToJson(UniValue& obj) const
if (ExtractDestination(scriptOperatorPayout, dest)) {
CBitcoinAddress bitcoinAddress(dest);
obj.push_back(Pair("operatorPayoutAddress", bitcoinAddress.ToString()));
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptOperatorPayout);
if (!strScriptPayout.empty())
obj.push_back(Pair("operatorPayoutAddress", strScriptPayout));
}
obj.push_back(Pair("inputsHash", inputsHash.ToString()));
}
Expand All @@ -476,6 +494,10 @@ std::string CProUpRegTx::ToString() const
std::string payee = "unknown";
if (ExtractDestination(scriptPayout, dest)) {
payee = CBitcoinAddress(dest).ToString();
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptPayout);
if (!strScriptPayout.empty())
payee = strScriptPayout;
}

return strprintf("CProUpRegTx(nVersion=%d, proTxHash=%s, pubKeyOperator=%s, votingAddress=%s, payoutAddress=%s)",
Expand All @@ -493,6 +515,10 @@ void CProUpRegTx::ToJson(UniValue& obj) const
if (ExtractDestination(scriptPayout, dest)) {
CBitcoinAddress bitcoinAddress(dest);
obj.push_back(Pair("payoutAddress", bitcoinAddress.ToString()));
} else {
std::string strScriptPayout = spark::ToStringSparkAddress(scriptPayout);
if (!strScriptPayout.empty())
obj.push_back(Pair("payoutAddress", strScriptPayout));
}
obj.push_back(Pair("pubKeyOperator", pubKeyOperator.ToString()));
obj.push_back(Pair("inputsHash", inputsHash.ToString()));
Expand Down
16 changes: 12 additions & 4 deletions src/libspark/coin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Coin::Coin(
this->serial_context = serial_context;

// Validate the type
if (type != COIN_TYPE_MINT && type != COIN_TYPE_SPEND) {
if (type != COIN_TYPE_MINT && type != COIN_TYPE_SPEND && type != COIN_TYPE_COINBASE) {
throw std::invalid_argument("Bad coin type");
}
this->type = type;
Expand Down Expand Up @@ -60,7 +60,7 @@ Coin::Coin(
//


if (this->type == COIN_TYPE_MINT) {
if (this->type == COIN_TYPE_MINT || this->type == COIN_TYPE_COINBASE) {
this->v = v;
// Encrypt recipient data
MintCoinRecipientData r;
Expand All @@ -81,6 +81,9 @@ Coin::Coin(
r_stream << r;
this->r_ = AEAD::encrypt(address.get_Q1()*SparkUtils::hash_k(k), "Spend coin data", r_stream);
}

if (this->type == COIN_TYPE_COINBASE)
this->k = k;
}

// Validate a coin for identification
Expand Down Expand Up @@ -123,7 +126,7 @@ IdentifiedCoinData Coin::identify(const IncomingViewKey& incoming_view_key) {
IdentifiedCoinData data;

// Deserialization means this process depends on the coin type
if (this->type == COIN_TYPE_MINT) {
if (this->type == COIN_TYPE_MINT || this->type == COIN_TYPE_COINBASE) {
MintCoinRecipientData r;

try {
Expand Down Expand Up @@ -154,7 +157,7 @@ IdentifiedCoinData Coin::identify(const IncomingViewKey& incoming_view_key) {
} catch (const std::exception &) {
throw std::runtime_error("Unable to identify coin");
}

// Check that the memo length is valid
unsigned char memo_length = r.padded_memo[0];
if (memo_length > this->params->get_memo_bytes()) {
Expand Down Expand Up @@ -221,4 +224,9 @@ void Coin::setParams(const Params* params) {
this->params = params;
}

bool Coin::isValidMNPayment(const spark::Address& addr, const std::vector<unsigned char>& serialContext) const {
Coin c(this->params, COIN_TYPE_COINBASE, k, addr, v, "BlockReward", serial_context);
return this->getHash() == c.getHash();
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add type validation and consider configurable memo

  1. Add validation to ensure this method is only called on coinbase coins:
 bool Coin::isValidMNPayment(const spark::Address& addr, const std::vector<unsigned char>& serialContext) const {
+    if (this->type != COIN_TYPE_COINBASE) {
+        return false;
+    }
     Coin c(this->params, COIN_TYPE_COINBASE, k, addr, v, "BlockReward", serial_context);
     return this->getHash() == c.getHash();
 }
  1. Consider making the "BlockReward" memo configurable through parameters to support different types of rewards or localization.

Committable suggestion skipped: line range outside the PR's diff.


}
15 changes: 13 additions & 2 deletions src/libspark/coin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ using namespace secp_primitives;
// Flags for coin types: those generated from mints, and those generated from spends
const char COIN_TYPE_MINT = 0;
const char COIN_TYPE_SPEND = 1;
const char COIN_TYPE_COINBASE = 2;

struct IdentifiedCoinData {
uint64_t i; // diversifier
Expand Down Expand Up @@ -93,6 +94,10 @@ class Coin {

void setParams(const Params* params);
void setSerialContext(const std::vector<unsigned char>& serial_context_);

// this is used ONLY to check masternode payout address validity,
bool isValidMNPayment(const spark::Address& addr, const std::vector<unsigned char>& serialContext) const;

protected:
bool validate(const IncomingViewKey& incoming_view_key, IdentifiedCoinData& data);

Expand All @@ -102,6 +107,7 @@ class Coin {
GroupElement S, K, C; // serial commitment, recovery key, value commitment
AEADEncryptedData r_; // encrypted recipient data
uint64_t v; // value
Scalar k; // nonce, is serialized only for coinbase o
std::vector<unsigned char> serial_context; // context to which the serial commitment should be bound (not serialized, but inferred)

// Serialization depends on the coin type
Expand All @@ -110,7 +116,7 @@ class Coin {
inline void SerializationOp(Stream& s, Operation ser_action) {
// The type must be valid
READWRITE(type);
if (type != COIN_TYPE_MINT && type != COIN_TYPE_SPEND) {
if (type != COIN_TYPE_MINT && type != COIN_TYPE_SPEND && type != COIN_TYPE_COINBASE) {
throw std::invalid_argument("Cannot deserialize coin due to bad type");
}
READWRITE(S);
Expand All @@ -125,7 +131,7 @@ class Coin {
if (ser_action.ForRead()) {
this->params = spark::Params::get_default();
}
if (type == COIN_TYPE_MINT && r_.ciphertext.size() != (1 + AES_BLOCKSIZE) + SCALAR_ENCODING + (1 + params->get_memo_bytes() + 1)) {
if ((type == COIN_TYPE_MINT || type == COIN_TYPE_COINBASE) && r_.ciphertext.size() != (1 + AES_BLOCKSIZE) + SCALAR_ENCODING + (1 + params->get_memo_bytes() + 1)) {
throw std::invalid_argument("Cannot deserialize mint coin due to bad encrypted data");
}
if (type == COIN_TYPE_SPEND && r_.ciphertext.size() != 8 + (1 + AES_BLOCKSIZE) + SCALAR_ENCODING + (1 + params->get_memo_bytes() + 1)) {
Expand All @@ -135,6 +141,11 @@ class Coin {
if (type == COIN_TYPE_MINT) {
READWRITE(v);
}

if (type == COIN_TYPE_COINBASE) {
READWRITE(v);
READWRITE(k);
}
}
};

Expand Down
10 changes: 10 additions & 0 deletions src/libspark/keys.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,14 @@ unsigned char Address::decode(const std::string& str) {
return network;
}

std::vector<unsigned char> Address::toByteVector(const unsigned char network) const {
std::string strAddr = encode(network);
return std::vector<unsigned char>(strAddr.begin(), strAddr.end());
}

unsigned char Address::fromByteVector(const std::vector<unsigned char>& vch) {
std::string strAddr(vch.begin(), vch.end());
return decode(strAddr);
}

}
3 changes: 3 additions & 0 deletions src/libspark/keys.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ class Address {
std::string encode(const unsigned char network) const;
unsigned char decode(const std::string& str);

std::vector<unsigned char> toByteVector(const unsigned char network) const;
Copy link

Choose a reason for hiding this comment

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

Top-level const qualifiers are meaningless on parameter types of non-definition function declarations. Please just have unsigned char network here.

unsigned char fromByteVector(const std::vector<unsigned char>& vch);

private:
const Params* params;
std::vector<unsigned char> d;
Expand Down
2 changes: 1 addition & 1 deletion src/libspark/mint_transaction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ MintTransaction::MintTransaction(
k.randomize();
this->coins.emplace_back(Coin(
this->params,
COIN_TYPE_MINT,
output.type,
k,
output.address,
output.v,
Expand Down
1 change: 1 addition & 0 deletions src/libspark/mint_transaction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ struct MintedCoinData {
Address address;
uint64_t v;
std::string memo;
char type;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Document the type field and consider using an enum class.

The newly added type field lacks documentation about its purpose and valid values. Additionally, using a raw char for type categorization could lead to type safety issues.

Consider the following improvements:

+// Defines the type of minted coin (e.g., regular mint, coinbase)
+enum class CoinType : char {
+    MINT = 0,
+    COINBASE = 1
+};
+
 struct MintedCoinData {
     Address address;
     uint64_t v;
     std::string memo;
-    char type;
+    CoinType type;  // Type of the minted coin
 };

This change would:

  1. Provide type safety through enum class
  2. Document the field's purpose
  3. Make valid values explicit
  4. Maintain the same memory footprint
📝 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
char type;
// Defines the type of minted coin (e.g., regular mint, coinbase)
enum class CoinType : char {
MINT = 0,
COINBASE = 1
};
struct MintedCoinData {
Address address;
uint64_t v;
std::string memo;
CoinType type; // Type of the minted coin
};

};

class MintTransaction {
Expand Down
Loading
Loading