Raise coinstake output count limit to 8

[tomasbrod]

#1256
There is still some debate whether to impose any limit at all.
whether to also limit the size of output scripts and size of hashBoinc

[denravonska]

Do we verify the total sum elsewhere?

[jamescowens]

Yes. SplitCoinStakeOutput will not allow more than 8 for V10. In #1244 I limited it to 2 in GetNumberOfStakeOutputs, which is called by SplitCoinStakeOutput, but the limit more properly belongs in SplitCoinStakeOutput.

[jamescowens]

Personally, I think for the coinstake transaction, we need to replace the above old code with a new check that verifies that the sum of all vout.nValues on the coinstake transaction minus the input.nValue (the prior output being used as the stake input) == bb.ResearchSubsidy. Maybe we don't do it here, but in CheckProofofResearch instead?

AcceptBlock currently calls CheckProofofResearch, which in turn deserializes the hashboinc into the object bb, and then compares the ResearchSubsidy in the hashboinc (bb.researchsubsidy) to the OUT_POR computed by GetProofofStakeReward. (This is where the 1.25 slop factor comes into play.)

GetProofofStakeReward does not currently appear to check whether the actual coinstake outputs minus the input equals bb.ResearchSubsidy. It just checks whether bb.ResearchSubsidy is sane compared to the calculations of GetProofofStakeReward. Maybe this is done currently and I missed it. It certainly needs to be done with sidestaking being allowed. What guarantee is there that the actual CBR + POR provided in the coinstake is the same as that recorded in the hashboinc?

[tomasbrod]

Hopefully it is checked somewhere at all. idk. We need to find it. I am pretty sure it is.

[jamescowens]

I think we are ok... here is main.cpp:3167 (although this part is changing with this PR)

        if (tx.IsCoinStake())
        {
            nStakeReward = nTxValueOut - nTxValueIn;
            if (tx.vout.size() > 3 && pindex->nHeight > nGrandfather) bIsDPOR = true;
            // ResearchAge: Verify vouts cannot contain any other payments except coinstake: PASS (GetValueOut returns the sum of all spent coins in the coinstake)
            if (IsResearchAgeEnabled(pindex->nHeight) && fDebug10)
            {
                int64_t nTotalCoinstake = 0;
                for (unsigned int i = 0; i < tx.vout.size(); i++)
                {
                    nTotalCoinstake += tx.vout[i].nValue;
                }
                if (fDebug10)   LogPrintf(" nHeight %d; nTCS %f; nTxValueOut %f",
                    pindex->nHeight,CoinToDouble(nTotalCoinstake),CoinToDouble(nTxValueOut));
            }

and then main.cpp:3222

double dStakeReward = CoinToDouble(nStakeReward+nFees) - DPOR_Paid; //DPOR Recipients checked above already
double dStakeRewardWithoutFees = CoinToDouble(nStakeReward) - DPOR_Paid;

and finally main.cpp:3329

            if ((bb.ResearchSubsidy + bb.InterestSubsidy + dDrift) < dStakeRewardWithoutFees)
            {
                    return DoS(20, error("ConnectBlock[] : Researchers Interest %f + Research %f + TimeDrift %f and total Mint %f, [StakeReward] <> %f, with Out_Interest %f, OUT_POR %f, Fees %f, DPOR %f  for CPID %s does not match calculated research subsidy",
                        (double)bb.InterestSubsidy,(double)bb.ResearchSubsidy,dDrift,CoinToDouble(mint),dStakeRewardWithoutFees,
                        (double)OUT_INTEREST,(double)OUT_POR,CoinToDouble(nFees),(double)DPOR_Paid,bb.cpid.c_str()));

            }

Note that since the construction of nStakeReward is from the sum of coinstake txn inputs - sum of coinstake txn outputs, it is already doing what I suggested, and is insensitive to the number of outputs, only the value.

[jamescowens]

Duh... I found the same stuff you posted below... anyway we are good...

[iFoggz]

line 3172 will make bIsDPOR = true if over 3 in vout size. i experienced this on testnet causing it to say i was being paid to much when trying to split my interest. u need to change it to
if (tx.vout.size() > 3 && pindex->nHeight > nGrandfather && pindex->nResearchSubsidy > 0) bIsDPOR = true;

[iFoggz]

or change it to different approach either or it assumed my interest block was POR there

[iFoggz]

also note oddly it seems we never reach a vout.size() > 3 is only settin bIsDPOR = true when we have 3 outputs. thou we dont seem to do that in POR/POS its usually 2 so that area needs some changes beyond what i've mentioned.

[iFoggz]

possible removal of it would suffice @denravonska

[tomasbrod]

• Gridcoin-Research/src/main.cpp

  Line 3171 in 7f6dca8

  nStakeReward = nTxValueOut - nTxValueIn;

• Gridcoin-Research/src/main.cpp

  Line 3332 in 7f6dca8

  if ((bb.ResearchSubsidy + bb.InterestSubsidy + dDrift) < dStakeRewardWithoutFees)

[iFoggz]

https://github.com/gridcoin-community/Gridcoin-Research/blob/development/src/qt/transactionrecord.cpp#L95 would this be affected by this change

[iFoggz]

That part shud be removed anyway lottery non existent now

[denravonska]

Pushed to staging as well.