Page MenuHomePhabricator

Added future-proofing for supporting 'Cash magic' in serialized data streams
AbandonedPublic

Authored by CCulianu on Aug 4 2017, 10:35.

Details

Reviewers
freetrader
deadalnix
zander
Group Reviewers
Restricted Project
Summary

More cash magic future-proofing a-la D400. Basically in case we want to completely remove legacy magic from everywhere, this code will support it going forward.

Test Plan

make check -- if tests pass, we're ok

Diff Detail

Repository
rABC Bitcoin ABC
Branch
morecashmagic
Lint
Lint Passed
SeverityLocationCodeMessage
Auto-Fixsrc/addrdb.cpp:1CFMTCode style violation
Auto-Fixsrc/test/net_tests.cpp:1CFMTCode style violation
Auto-Fixsrc/validation.cpp:1CFMTCode style violation
Unit
No Test Coverage
Build Status
Buildable 645
Build 645: arc lint + arc unit

Event Timeline

Eh, reformat some funny formattig create by the linter

I realize that the linter is complaining -- honestly I disagree with its suggetions.

Let me know if that's a problem going forward -- like if this project strongly enforces always listening to the linter for code style. If so, I will comply with the project standard. (Just wasn't sure how hard and fast the rule is enforced for this project, so I am testing it with this! ;) )

deadalnix requested changes to this revision.Aug 4 2017, 11:25

We can bypass all that by having a script read the old folder and move it all into a new one with the new magic and we solve 2 problems at once.

On the code itself, it says you have style errors, so requesting changes until this is sorted out.

This revision now requires changes to proceed.Aug 4 2017, 11:25

We use clang-format 3.8 so far. Later version somewhat differs sometime.

We can bypass all that by having a script read the old folder and move it all into a new one with the new magic and we solve 2 problems at once.

On the code itself, it says you have style errors, so requesting changes until this is sorted out.

So I take this to mean, as a general rule, diffs need to not have format errors, then....

*grumble* *grumble*.. ok.. I'll supplicate myself to the linter, I guess. *grumble*

:P

CCulianu edited edge metadata.

Trying to fix code format issues

We can bypass all that by having a script read the old folder and move it all into a new one with the new magic and we solve 2 problems at once.

Agreed -- or even a startup/init routine that detects old files and converts them? Either/or...

yes but, then I'm not sure it make sense to support double magic there. After migration only one magic will be used. The convert script will be the one handling double magic.

deadalnix requested changes to this revision.Aug 4 2017, 16:54

Also there is no test. I think we should put this asside,this is not urgent anyway because it is useless as long as we don't have many node in the wild compatible with the new magic, and we won't have any if we don't release.

This revision now requires changes to proceed.Aug 4 2017, 16:54

Ok, fair enough -- the convert script it will be, then. Makes sense actually to just go that route -- when authoring this I wasn't aware of convert script idea.