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.

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

Remove payments CSV export. #759

Merged

Conversation

denravonska
Copy link
Member

The implementation made it really difficult to reorganize to a new chain as the payments would have to be cut out from a concatenated string, see #756. The information is still in the chain but those who wants to export the data to CSV will have to traverse the chain manually.

The implementation made it really difficult to reorganize
to a new chain as the payments would have to be cut out
from a concatenated string. The information is still in the
chain but those who wants to export the data to CSV will
have to traverse the chain manually.
// it until it's used.
std::string sReserved;
READWRITE(sReserved);
std::string dummy;
Copy link
Member Author

Choose a reason for hiding this comment

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

Better way of doing this? :( Fiddled with this before. We could serialize based on block version but that removes the backward compatibility (as in when downgrading).

Copy link
Member

Choose a reason for hiding this comment

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

It is very difficult to work with the IMPLEMENT_SERIALIZE and READWRITE macros. The underlying template code is nice.
Maybe these could be modified or broken down into separate read and write and then go with conditions.

Copy link
Member Author

@denravonska denravonska Dec 11, 2017

Choose a reason for hiding this comment

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

Yeah, I was thinking something like:

if(nVersion < 9) {
   // Read/write obsolete GRC address and reserved string
   std::string dummy;
   READWRITE(dummy); // Address
   READWRITE(dummy); // Reserved string
}

This will fail when downgrading if you have a chain written by a new client.

//printf(" {CPIDs Re-Loaded} ");
msNeuralResponse="";
}
msNeuralResponse.clear();
Copy link
Member

Choose a reason for hiding this comment

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

why you removed the barackets?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because one-line brackets feel too fluffy :)

@@ -5247,11 +5229,6 @@ void AddResearchMagnitude(CBlockIndex* pIndex)
stMag.interestPayments += pIndex->nInterestSubsidy;

AdjustTimestamps(stMag,pIndex->nTime, pIndex->nResearchSubsidy);
// Track detailed payments made to each CPID
stMag.PaymentTimestamps += ToString(pIndex->nTime) + ",";
stMag.PaymentAmountsResearch += RoundToString(pIndex->nResearchSubsidy,2) + ",";
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this field is not needed for the reward calculation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it wouldn't build otherwise as the fields were removed from the struct as well.

@denravonska denravonska merged commit 678f1ff into gridcoin-community:staging Dec 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants