0.21.kyo.2 #50

akyo8 · 2021-02-20T08:32:47Z

The issue with 'secp256k1_ecdsa_verify' failing when verifying transaction signature hashes has now been resolved. The pubkey signature check has now been re-enabled. This is currently a more complete and stable version of the codebase and should be pulled upstream please.

Feel like adding this is a step back if we go with SegWit enabled at a later point. I actually went and looked at the oldballs Bitcoin code and compared it to Cloak2 implementation.

Bitcoin:
https://github.com/bitcoin/bitcoin/blob/881a85a22d76c875f519cd54388a419ec6f70857/src/script.cpp (before the inline serializer commit)

"Inline signature serializer

Instead of building a full copy of a CTransaction being signed, and
then modifying bits and pieces until its fits the form necessary
for computing the signature hash, use a wrapper serializer that
only serializes the necessary bits on-the-fly.

This makes it easier to see which data is actually being hash,
reduces load on the heap, and also marginally improves performances
(around 3-4us/sigcheck here). The performance improvements are much
larger for large transactions, though.

The old implementation of SignatureHash is moved to a unit tests,
to test whether the old and new algorithm result in the same value
for randomly-constructed transactions."

Cloak:
https://github.com/CloakProject/CloakCoin/blob/master/src/script.cpp

Seems like there is nothing different in how the hash is constructed tbh, so this might be unnecessary. In fact, the only difference is that Bitcoin uses

CHashWriter ss(SER_GETHASH, 0); ss << txTmp << nHashType; return ss.GetHash();

while Cloak has

CDataStream ss(SER_GETHASH, 0); ss.reserve(10000); ss << txTmp << nHashType; return Hash(ss.begin(), ss.end());

After doing more research into SegWit details, I am very inclined to not even consider SegWit implementation in the Cloak code due to possible min(t)er previous block signatures retention:

https://www.youtube.com/watch?v=VoFb3mcxluY
https://twitter.com/peterktodd/status/881206218659946496
https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-December/012103.html

Transaction malleability should be resolved in a non-SegWit way. ACK and approved.

Update hash.h to include X13 functionality Validate

Add PoS placeholder and params Update clientversion

Change MAKX_MONEY value from 21M to 10M. Sync improvements for older clients. TimeHash wrapper for stake modifier hash timestamps (fixes old uint256 sorting issue). Some further work to stake modifier genetion (WIP) Added some debug code for debugging stake modier generation :q q

…nNet

…sions

A lot more proof-of-stake code available. need debugging for syncing. Note : CPubKey::Verify currently hardcoded to return TRUE always. This is done sidestpe a sep256k headache should be addressed! Note : GetStakeModifiedCheckSum currently producint wrong value.

Non-segwit transaction (currently all transactions) are now hashed using the legacy code from previous Cloak code base. This ensures a matching TX comparison hash is always generated for use with TX signature hash validation. It is doubtful that this code will need to change in fture, but if it is decided that the verification method needs to change, then a network fork will need to be performed so that newer blocks use a new TX signature hash.

anorakthagreat · 2021-03-13T21:18:46Z

@akyo8 : You are correct, the issues you highlighted with SegWit is the exact reason why it was not included in the codebase in the first place. Especially point #5 in the video of Dr. Rizun and the thought experiment in the end, is concerning.
We will not be implementing SegWit in Cloak.

anorakthagreat · 2021-03-13T21:46:10Z

@akyo8 REM build_msvc/libbitcoinconsensus/Source.cpp

anorakthagreat · 2021-03-13T21:54:57Z

src/amount.h

@@ -22,7 +22,7 @@ static const CAmount COIN = 100000000;
 * critical; in unusual circumstances like a(nother) overflow bug that allowed
 * for the creation of coins out of thin air modification could lead to a fork.
 * */


what happened to
static const CAmount CENT = 1000000;
I believe this was used across different files?

anorakthagreat · 2021-03-13T22:42:53Z

src/chain.h

@@ -63,6 +67,12 @@ class CBlockFileInfo
         nBlocks = 0;
         nSize = 0;
         nUndoSize = 0;
+         nFlags = 0;


These changes belong in the CBlockIndex class, not here in CBlockFileInfo. Please move these.

Also, CBlockIndex is completely missing the SetNull method, which is called on COutPoint prevoutStake across the file and possibly elsewhere.

Take a look at the file on 0.17 here:
https://github.com/CloakProject/codename-phoenix/blob/0.17.cloak/src/chain.h

Please make sure you're careful when you move code, it needs to be added to appropriate locations - this is important as some properties are purposely mem-only or not serialized at all depending on usage. I understand you were trying to short-hand initialize the values on declaration with {0}, but this has other implications, security and byte-size included (declaring a var doesn't reserve memory, sometimes there is no stack at all depending on the compiler optimization), so some classes don't need all the properties or we don't want them on disk.

anorakthagreat · 2021-03-13T23:13:16Z

src/chain.h

 public:
    uint256 hashPrev;

    CDiskBlockIndex() {
        hashPrev = uint256();
+


missing blockHash = uint256();

anorakthagreat · 2021-03-14T07:10:41Z

src/chainparams.cpp

@@ -67,74 +70,131 @@ class CMainParams : public CChainParams {
        consensus.signet_blocks = false;
        consensus.signet_challenge.clear();
        consensus.nSubsidyHalvingInterval = 210000;
-        consensus.BIP16Exception = uint256S("0x00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22");
+        consensus.BIP16Exception = uint256S("0x00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22"); // P2SH
        consensus.BIP34Height = 227931;


rem: consensus.BIP34Height = 227931; (redefined below)

anorakthagreat · 2021-03-14T07:12:02Z

src/chainparams.cpp

        consensus.BIP34Height = 227931;
+        consensus.BIP34Height = 0;
        consensus.BIP34Hash = uint256S("0x000000000000024b89b42a942fe0d9fea3bb44ab7bd1b19115dd6a759c0808b8");


rem: consensus.BIP34Hash = uint256S("0x000000000000024b89b42a942fe0d9fea3bb44ab7bd1b19115dd6a759c0808b8"); (redefined below)

anorakthagreat · 2021-03-14T07:27:24Z

src/chainparams.cpp

-        consensus.powLimit = uint256S("00000000ffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
-        consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks
+        consensus.BIP34Hash = uint256S("0x2d8251121940abce6e28df134c6432e8c5a00d59989a2451806c2778c3a06112"); // Block v2, Height in Coinbase [using genesis]
+        consensus.BIP65Height = 388381;                                                                       // 000000000000000004c2b624ed5d7756c508d90fd0da2c7c679febfa6c4735f0	        consensus.BIP65Height = 388381; // OP_CHECKLOCKTIMEVERIFY [Consensus (soft fork)] - forced far into future for now


Note: We will want BIP65 implemented and functional in the new codebase, CheckLockTimeVerify is a crucial and paramount piece needed for timed locks and ultimately - atomic swaps (along with P2SH).
I know for a fact CLTV wasn't fully implemented in the current codebase, IIRC some pieces of code were not pulled in from the newer Bitcoin core. This will need a closer/detailed look.

BIP65 might need to be set to something in the future as an activation height to enable CLTV & atomic swaps, but maybe we can leave it as is so it's immediately active when implemented (older clients will have this value already)
The value here is from btc core and way back in the past (Cloak has 60s block time).

anorakthagreat · 2021-03-14T08:02:00Z

src/chainparams.cpp

+        consensus.BIP65Height = 388381;                                                                       // 000000000000000004c2b624ed5d7756c508d90fd0da2c7c679febfa6c4735f0	        consensus.BIP65Height = 388381; // OP_CHECKLOCKTIMEVERIFY [Consensus (soft fork)] - forced far into future for now
+        consensus.BIP66Height = 363725;                                                                       // 00000000000000000379eaa19dce8c9b722d46ae6a57c2f1a988119488b50931	        consensus.BIP66Height = 363725; // Strict DER signatures [Consensus (soft fork)] - forced far into future for now
+        consensus.nProofOfStakeLimit = UintToArith256(uint256S("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"));
+        consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks	        consensus.nPowTargetTimespan = 3.5 * 24 * 60 * 60; // 3.5 days


rem:

consensus.nProofOfStakeLimit = UintToArith256(uint256S("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")); consensus.nPowTargetTimespan = 14 * 24 * 60 * 60; // two weeks

redefined below, values above are from btc core

anorakthagreat · 2021-03-14T08:12:10Z

src/chainparams.cpp

        pchMessageStart[0] = 0xf9;
+        pchMessageStart[0] = 0x70;
        pchMessageStart[1] = 0xbe;


duplicate definitions here as well...

anorakthagreat · 2021-03-14T08:12:33Z

src/chainparams.cpp

+         consensus.vDeployments[Consensus::DEPLOYMENT_CSV].bit = 0;
+        consensus.vDeployments[Consensus::DEPLOYMENT_CSV].nStartTime = 1462060800; // May 1st, 2016
+        consensus.vDeployments[Consensus::DEPLOYMENT_CSV].nTimeout = 1493596800;   // May 1st, 2017		
+        consensus.vDeployments[Consensus::DEPLOYMENT_CSV].nTimeout = 1809129600;   // May 1st, 2027



This whole section is a mess, with duplicate definitions and everything moved out of place.
This needs cleanup. See

codename-phoenix/src/chainparams.cpp

Line 79 in 5e18c09

CMainParams() {

anorakthagreat · 2021-03-14T08:16:49Z

src/chainparams.cpp

-        consensus.powLimit = uint256S("00000377ae000000000000000000000000000000000000000000000000000000");
-        consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
+
+		onsensus.nProofOfStakeLimit = UintToArith256(uint256S("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"));


c missing?
cmon man...

anorakthagreat · 2021-03-14T08:19:29Z

src/chainparams.cpp

@@ -443,7 +563,7 @@ class CRegTestParams : public CChainParams {
        base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF};
        base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94};

-        bech32_hrp = "bcrt";
+        bech32_hrp = "ccrt";
    }

    /**


This whole file is a mess, needs to be reverted to btc core version and Cloak specific values applied from: https://github.com/CloakProject/codename-phoenix/blob/0.17.cloak/src/chainparams.cpp

anorakthagreat · 2021-03-15T03:01:36Z

src/chainparams.h

@@ -91,6 +91,11 @@ class CChainParams
    const std::vector<SeedSpec6>& FixedSeeds() const { return vFixedSeeds; }
    const CCheckpointData& Checkpoints() const { return checkpointData; }
    const ChainTxData& TxData() const { return chainTxData; }
+    const arith_uint256& ProofOfWorkLimit() const { return consensus.nProofOfWorkLimit; }


missing:

// MODIFIER_INTERVAL: time to elapse before new modifier is computed static const unsigned int MODIFIER_INTERVAL = 5 * 60; // * 60; // 20 minutes // MODIFIER_INTERVAL_RATIO: // ratio of group interval length between the last group and the first group static const int MODIFIER_INTERVAL_RATIO = 3;

anorakthagreat · 2021-03-15T03:05:16Z

src/consensus/params.h

+    arith_uint256 nProofOfStakeLimit;
+    unsigned int nStakeMinAge;
+    unsigned int nStakeMaxAge;
+    unsigned int nStakeTargetSpacing;     


missing:

unsigned int nStakeModifierInterval; unsigned int nCoinbaseMaturity;

akyo8 added 22 commits February 18, 2021 12:04

⚡ Add Enigma annoucement message skeleton \n Fix up PoW validation

1a85d7c

Multi-line

b840689

Update hash.h to include X13 functionality Validate

Update CBlockIndex for PoS. Fix coinbase maturity in consensus rules

40b2167

Fix up fees and block extra connections when using -onnect

1a00326

Update PoW check to ignore genesis block

687bc08

⚡ Add PoS placeholder and params. Update clientversion.

850f5c6

Add PoS placeholder and params Update clientversion

Update block, transaction and uint256 (add uint512)

749652f

Update version.h

1de3eec

Update chainparams

e762b6d

Add X13 crypto sources to makefile

c5c2338

Add X13 crypto files

c8e1daf

Fix some sync issues

5aa90a6

Add more proof-of-stake code and re-add canHandlePoSHeaders logic

90750ea

Fix nProofOfWorkLimit and nProofOfStakeLimit consensus values for mai…

73daa61

…nNet

Add crypto/hmac_sha512.h to src/Makefile.am

937041a

Fix GetStakeModifierChecksum and removed unwanted uint256 arith exten…

ad37d7a

…sions

Add Cloak references

be75d75

Update some Cloak/BTC references

71f5454

squash commit

25cfe3a

anorakthagreat reviewed Mar 13, 2021

View reviewed changes

src/chain.h

public:

uint256 hashPrev;

CDiskBlockIndex() {

hashPrev = uint256();

Copy link

Collaborator

anorakthagreat Mar 13, 2021

Choose a reason for hiding this comment

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

missing blockHash = uint256();

anorakthagreat reviewed Mar 14, 2021

View reviewed changes

anorakthagreat reviewed Mar 15, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

0.21.kyo.2 #50

0.21.kyo.2 #50

akyo8 commented Feb 20, 2021

anorakthagreat commented Mar 13, 2021 •

edited

Loading

anorakthagreat commented Mar 13, 2021

anorakthagreat Mar 13, 2021

anorakthagreat Mar 13, 2021 •

edited

Loading

anorakthagreat Mar 13, 2021

anorakthagreat Mar 14, 2021 •

edited

Loading

anorakthagreat Mar 14, 2021

anorakthagreat Mar 14, 2021 •

edited

Loading

anorakthagreat Mar 14, 2021

anorakthagreat Mar 14, 2021

anorakthagreat Mar 14, 2021

anorakthagreat Mar 14, 2021

anorakthagreat Mar 14, 2021

anorakthagreat Mar 15, 2021

anorakthagreat Mar 15, 2021

0.21.kyo.2 #50

Are you sure you want to change the base?

0.21.kyo.2 #50

Conversation

akyo8 commented Feb 20, 2021

anorakthagreat commented Mar 13, 2021 • edited Loading

anorakthagreat commented Mar 13, 2021

Choose a reason for hiding this comment

anorakthagreat Mar 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anorakthagreat Mar 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anorakthagreat Mar 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anorakthagreat commented Mar 13, 2021 •

edited

Loading

anorakthagreat Mar 13, 2021 •

edited

Loading

anorakthagreat Mar 14, 2021 •

edited

Loading

anorakthagreat Mar 14, 2021 •

edited

Loading