Display of research rewards in the wallet reverts to interest with sidestaking with less than 100% allocated

[jamescowens]

I discovered this while testing the sidestake PR #1265. This is related to issue #1256 and also PR #1261.

My testing of the sidestake PR is successful. Specifying a valid address and allocation percentage to an address external to the wallet properly sends the desired reward to the recipient.

The issue is how it is displayed. If less than 100% of the reward is allocated, the distribution calculation "breaks" the 123/4 special coding to indicate to IsPoR() that it is a research reward and it shows as mined interest on both the sending wallet and the receiving wallet rather than mined - POR.

I think the problem revolves around two issues:

1. The use of the special encoding of the 123/4 in the reward calculation which is then used by the IsPoR() function to determine whether the amount is a reward or not, which I really don't like...

bool IsPoR(double amt)
{
std::string sAmt = RoundToString(amt,8);
if (sAmt.length() > 8)
{
std::string suffix = sAmt.substr(sAmt.length()-4,4);
if (suffix =="0124" || suffix=="0123")
{
return true;
}
}
return false;
}

and

2. In the TransactionTableModel::formatTxType the use of wtx->credit + wtx->debit to detect the 124 or 123...

QString TransactionTableModel::formatTxType(const TransactionRecord *wtx) const
{
switch(wtx->type)
{
case TransactionRecord::RecvWithAddress:
return tr("Received with");
case TransactionRecord::RecvFromOther:
return tr("Received from");
case TransactionRecord::SendToAddress:
case TransactionRecord::SendToOther:
return tr("Sent to");
case TransactionRecord::SendToSelf:
return tr("Payment to yourself");
case TransactionRecord::Generated:
if (wtx->RemoteFlag==1 && false)
{

		double reward = CoinToDouble(wtx->credit + wtx->debit);
		double max = GetMaximumBoincSubsidy(GetAdjustedTime());
		if (reward==max)
		{
			return tr("Mined - DPOR");
		}
		else
		{
			return tr("Minted - (Local) DPOR");
		}
	}
	else if (((IsPoR(CoinToDouble(wtx->credit + wtx->debit)))))
	{
			return tr("Mined - PoR");
	}
	else
	{
			return tr("Mined - Interest");
	}
default:
    return QString();
}

}

which I think breaks under the distribution of less than 100% of the rewards...

Also we have in transactionrecord.cpp the following for TransactionRecord::decomposeTransaction(const CWallet *wallet, const CWalletTx &wtx) starting at line 89...

            if (wtx.IsCoinStake())
            {
                // Generated (proof-of-stake)
		        if (hashPrev == hash)
                    continue; // last coinstake output

				if (wtx.vout.size()==2)
				{
					//Standard POR CoinStake
					sub.type = TransactionRecord::Generated;
					sub.credit = nNet > 0 ? nNet : wtx.GetValueOut() - nDebit;
					hashPrev = hash;
				}
				else
				{
                    // This part used to be used for a deprecated "crypto lottery". It is now
                    // necessary for the implementation of side staking in PR 1265.
					sub.type = TransactionRecord::Generated;
					if (nDebit == 0)
					{
						sub.credit = GetMyValueOut(wallet,wtx);
						sub.RemoteFlag = 1;
					}
					else
					{
						sub.credit = nNet > 0 ? nNet : GetMyValueOut(wallet,wtx) - nDebit;
					}

					hashPrev = hash;
				}
            }

This code attempts to compensate for the fact that in a situation where there are more than two coinstake outputs, there is likely to be one that has been "distributed" to a remote wallet. (What I call an allocation.) This code attempts to fix up the display of the net value properly because the "debit" is going to be zero for the transaction on the remote side. This doesn't have anything to do with whether the transaction is right. It is. It is a display issue...

Comments. I know that @Foggyx420 is experimenting with solutions.

We should have a discussion of what should show up in the wallet GUI on rewards allocated to addresses outside of the staking wallet. A straightforward fix to the above would cause them to be displayed in the receiving wallet as Mined - POR. Right now they are being displayed as Mined - Interest unless 100% of the reward was distributed, which preserves the 123/4 encoding. I am comfortable with that, even though the receiving wallet did not actually do the generation of the funds, because the coins are still part of a minting transaction, so it is properly marked.

[iFoggz]

Ive been at this solution today and so far it is looking good. all ui stuff and icons are now based on the coinstake data not the 0123 0124. i'm having good success and think it will easily support this since it would be in the same tx coinstake (the splitting) plan to pr up today and we all can nit and pick at it.

[iFoggz]

i've eliminated the IsPoR in wallet.cpp so far

with success and acceptance of this yet to be PR will allow us to eliminate the 0123 0124 in miner as well

[tomasbrod]

#303

[tomasbrod]

Related to bounty task: gridcoin-community/Gridcoin-Tasks#176

[iFoggz]

i still think the side stake/split stake should be detectable as well so that another wallet receiving the split/side will show an appropriate icon in support of this. perhaps the ui re-designer could make us such icons.

[jamescowens]

I agree. We actually need four icons....

Mined - POR (exists)
Mined - Interest (exists)
Mined Transfer - POR (new)
Mined Transfer - Interest (new)

The two new icons could look similar to the original corresponding ones with a modification to indicate the transfer of the mined amount from another wallet.

[jamescowens]

PR to deal with this is merged. Closing.