testnet sync fix - bad blocks on exception list (again... learning git, lol) #1252

Quezacoatl1 · 2018-08-08T18:48:56Z

No description provided.

denravonska · 2018-08-13T04:26:52Z

src/main.cpp

+                           ,uint256("fc584b18239f3e3ea78afbbd33af7c6a29bb518b8299f01c1ed4b52d19413d4f") //T407214
+                           ,uint256("d5441f7c35eb9ea1b786bbbed820b7f327504301ae70ef2ac3ca3cbc7106236b") //T479114
+                        };
+                        if( vSkipHashdStakeRewardSignCheck.count(pindex->GetBlockHash())==0 )


We want to limit this check to testnet, right?

I agree, when I get the chance I will change the PR and add a check to the if.

denravonska · 2018-08-13T04:27:03Z

Set your editor's tab width to 4 spaces ;)

tomasbrod

Please change the skip condition to still execute the checks, but only LogPrintf warning instead of return error.
This way the diff should be smaller.
Please change the log message to something true, this is reward check not signature/hash check.

Quezacoatl1 · 2018-08-15T08:11:36Z

@tomasbrod will try to suit your needs

Quezacoatl1 · 2018-08-15T19:01:46Z

@denravonska @tomasbrod
I changed my tab width, the logic and the warning message. Sync from 0 is being tested. I hope I did not completely mess up/misunderstand the if statements as I am very very far from being certain about what I did :D Small steps...

tomasbrod · 2018-08-15T19:50:19Z

src/main.cpp

                        if (dStakeReward > ((OUT_POR*1.25)+OUT_INTEREST+1+CoinToDouble(nFees)))
                        {
                            StructCPID st1 = GetLifetimeCPID(pindex->GetCPID(),"ConnectBlock()");
-                            GetProofOfStakeReward(nCoinAge, nFees, bb.cpid, true, 2, nTime,
-                                        pindex, "connectblock_researcher_doublecheck", OUT_POR, OUT_INTEREST, dAccrualAge, dMagnitudeUnit, dAvgMagnitude);
+                            GetProofOfStakeReward(nCoinAge, nFees, bb.cpid, true, 2, nTime, pindex, "connectblock_researcher_doublecheck", OUT_POR, OUT_INTEREST, dAccrualAge, dMagnitudeUnit, dAvgMagnitude);


regression in readability

fixed, updating when I fully understood all your proposals

tomasbrod · 2018-08-15T19:50:49Z

src/main.cpp

+                                    if (fDebug3) LogPrintf("ConnectBlockError[ResearchAge] : Researchers Reward Pays too much : Interest %f and Research %f and StakeReward %f, OUT_POR %f, with Out_Interest %f for CPID %s ",
+                                        (double)bb.InterestSubsidy,(double)bb.ResearchSubsidy,dStakeReward,(double)OUT_POR,(double)OUT_INTEREST,bb.cpid);
+                                        return DoS(10,error("ConnectBlock[ResearchAge] : Researchers Reward Pays too much : Interest %f and Research %f and StakeReward %f, OUT_POR %f, with Out_Interest %f for CPID %s ",
+                                            (double)bb.InterestSubsidy,(double)bb.ResearchSubsidy,dStakeReward,(double)OUT_POR,(double)OUT_INTEREST,bb.cpid.c_str()));


misleading indent

I never touched those lines except for copying, can you elaborate what you mean?

The diff clearly shows you did.
I can fix it for you later.

omg I just did not know what an indent was. sorry

denravonska · 2018-08-16T04:45:12Z

src/main.cpp

-
-                                return DoS(10,error("ConnectBlock[ResearchAge] : Researchers Reward Pays too much : Interest %f and Research %f and StakeReward %f, OUT_POR %f, with Out_Interest %f for CPID %s ",
-                                    (double)bb.InterestSubsidy,(double)bb.ResearchSubsidy,dStakeReward,(double)OUT_POR,(double)OUT_INTEREST,bb.cpid.c_str()));
+                                if( vSkipHashdStakeRewardSignCheck.count(pindex->GetBlockHash())==0 || (vSkipHashdStakeRewardSignCheck.count(pindex->GetBlockHash())!=0 && !fTestNet))


These hashes only affect testnet, right? If so you can change it to ehter:

// Compact if(!fTestNet || vSkipHashdStakeRewardSignCheck.count(pindex->GetBlockHash()) !=0)

and maybe change the set name to badSignBlocksTestnet.

That way we don't accidentally skip a block in prod and we don't have to do the set lookup.

Yes, those exceptions are testnet only.

Changed the set name and compacted the if statement. Thanks!

Are you certain your if statement is correct? If I got it right (and I just copied form a few lines above within the code) I need

vSkipHashdStakeRewardSignCheck.count(pindex->GetBlockHash()) ==0

to check that the current block is NOT amongst the exceptions.

This is getting complicated. You only conditioning the error return. So you want to return error only if( (it is not testnet) or (the block is not in exception list) ).

iFoggz · 2018-08-18T19:51:12Z

src/main.cpp

+                                {
+                                    if (fDebug3) LogPrintf("ConnectBlockError[ResearchAge] : Researchers Reward Pays too much : Interest %f and Research %f and StakeReward %f, OUT_POR %f, with Out_Interest %f for CPID %s ",
+                                        (double)bb.InterestSubsidy,(double)bb.ResearchSubsidy,dStakeReward,(double)OUT_POR,(double)OUT_INTEREST,bb.cpid);
+                                        return DoS(10,error("ConnectBlock[ResearchAge] : Researchers Reward Pays too much : Interest %f and Research %f and StakeReward %f, OUT_POR %f, with Out_Interest %f for CPID %s ",


one ident too deep :p it happens. your learning; do not give up!

Quezacoatl1 · 2018-08-19T09:34:09Z

Guys thanks for your comments and reviews but I fear I am stealing you more time than useful because I am just not into coding. I suggest you take the hashes and quickly put those with a suitable if check and a good coding style in the code yourself ;)

denravonska · 2018-08-21T13:14:01Z

Replaced by #1268

@tomasbrod

Added: - Linux nodes can now stake superblocks using forwarded contracts #1060 (@tomasbrod). Changed: - Replace interest with constant block reward #1160 (@tomasbrod). Fork is set to trigger at block 1420000. - Raise coinstake output count limit to 8 #1261 (@tomasbrod). - Port of Bitcoin hash implementation #1208 (@jamescowens). - Minor canges for the build documentation #1091 (@Lenni). - Allow sendmany to be used without an account specified #1158 (@Foggyx420). Fixed: - Fix `cpids` and `validcpids` not returning the correct data #1233 (@Foggyx420). - Fix `listsinceblock` not showing mined blocks to change addresses #501 (@Foggyx420). - Fix crash when raining using a locked wallet #1236 (@Foggyx420). - Fix invalid stake reward/fee calculation (@jamescowens). - Fix divide by zero bug in `getblockstats` RPC #1292 (@Foggyx420). - Bypass historical bad blocks on testnet #1252 (@Quezacoatl1). - Fix MacOS memorybarrier warnings #1193 (@ghost). Removed: - Remove neuralhash from the getpeerinfo and node stats #1123 (@Foggyx420). - Remove obsolete NN code #1121 (@Foggyx420). - Remove (lower) Mint Limiter #1212 (@tomasbrod).

testnet sync fix - bad blocks on exception list

afdb2c6

denravonska reviewed Aug 13, 2018

View reviewed changes

denravonska added the bug label Aug 13, 2018

denravonska added this to the Camilla milestone Aug 13, 2018

tomasbrod suggested changes Aug 13, 2018

View reviewed changes

changing check logic and warning message

4fa4fee

tomasbrod reviewed Aug 15, 2018

View reviewed changes

denravonska reviewed Aug 16, 2018

View reviewed changes

Quezacoatl1 added 3 commits August 16, 2018 07:46

improved readability

c4e4aad

compacted if statement and renamed exception list

312a1bf

added comment

9fdec4f

iFoggz reviewed Aug 18, 2018

View reviewed changes

tomasbrod mentioned this pull request Aug 19, 2018

testnet sync fix - bad blocks on exception list #1268

Merged

denravonska closed this Aug 21, 2018

testnet sync fix - bad blocks on exception list (again... learning git, lol) #1252

testnet sync fix - bad blocks on exception list (again... learning git, lol) #1252

Uh oh!

Conversation

Quezacoatl1 commented Aug 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

denravonska commented Aug 13, 2018

Uh oh!

tomasbrod left a comment

Choose a reason for hiding this comment

Uh oh!

Quezacoatl1 commented Aug 15, 2018

Uh oh!

Quezacoatl1 commented Aug 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Quezacoatl1 commented Aug 19, 2018

Uh oh!

denravonska commented Aug 21, 2018

Uh oh!

Uh oh!