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

Change format of gobject, store/transmit vchData instead of hex-encoded string of a string #1934

Merged
merged 2 commits into from
Feb 15, 2018

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Feb 15, 2018

It really makes no sense to store/pass around hex string of a string... This should shrink data size to half its size as of now.

@UdjinM6 UdjinM6 added this to the 12.3 milestone Feb 15, 2018
Copy link

@codablock codablock left a comment

Choose a reason for hiding this comment

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

utACK with 2 small nits (I'm fine if you leave them)

@@ -22,7 +22,7 @@ CProposalValidator::CProposalValidator(const std::string& strDataHexIn)

void CProposalValidator::Clear()
{
strData = std::string();
strDataHex = std::string();

Choose a reason for hiding this comment

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

Just a nit: There is .clear() on std::string

@@ -31,7 +31,7 @@ void CProposalValidator::Clear()
void CProposalValidator::SetHexData(const std::string& strDataHexIn)
{
std::vector<unsigned char> v = ParseHex(strDataHexIn);
strData = std::string(v.begin(), v.end());
strDataHex = std::string(v.begin(), v.end());

Choose a reason for hiding this comment

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

Just a nit: There is .assign(begin, end) on std::string

@codablock
Copy link

Hmm, no protobump? Do you plan to first upgrade all nodes and then release a new version with a protobump?

@UdjinM6
Copy link
Author

UdjinM6 commented Feb 15, 2018

No protobump is needed, it's backwards compatible on p2p level just like #1933

Missed a thing in serialization, fab47d0 should fix it.

@codablock
Copy link

I see that it's backwards compatible, but as we keep the protoversion on 70208, the new code/signing will never trigger until we release a version with protoversion >= 70209.

But now that I'm thinking about it it starts to make sense. If we protobump now, old nodes will immediately start to ban new nodes.

@codablock
Copy link

re-utACK fab47d0

@UdjinM6
Copy link
Author

UdjinM6 commented Feb 15, 2018

Yes, I mean, it's implemented in such a way that we talk to old nodes in old format and to new nodes in new format i.e. it always serializes/deserializes correctly on a per node basis, even though it does it differently for different versions.

@UdjinM6 UdjinM6 merged commit aadec37 into dashpay:develop Feb 15, 2018
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
…ed string of a string (dashpay#1934)

* Change format of gobject, store/transmit vchData instead of hex-encoded string

* fix (limited string for old format)
CryptoCentric pushed a commit to absolute-community/absolute that referenced this pull request Feb 28, 2019
…ed string of a string (dashpay#1934)

* Change format of gobject, store/transmit vchData instead of hex-encoded string

* fix (limited string for old format)
CryptoCentric pushed a commit to absolute-community/absolute that referenced this pull request Mar 2, 2019
…ed string of a string (dashpay#1934)

* Change format of gobject, store/transmit vchData instead of hex-encoded string

* fix (limited string for old format)
@UdjinM6 UdjinM6 deleted the strdatatovchdata branch November 26, 2020 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants