-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
wallet2: make unsigned/signed tx portable #1507
Conversation
8ced8b2
to
1975186
Compare
I assume the BEGIN_SERIALIZE_OBJECT() bits aren't used on those types anymore ? Then plese remove them, or any changes to the structure will cause this to change, and cause the new ones to be forgot. If the BEGIN_SERIALIZE_OBJECT() isn't here, the coder will not think he's done the serialization changes. |
And bump the file format version. |
Removing it will make it impossible to read existing serialized files. If that's acceptable, I'll remove it.
OK |
That's fine. The version check would prevent this too. And if you don't bump the version, you end up with two incompatible sets of files with the same version number. |
1975186
to
098337a
Compare
Is it OK now? |
You did not bump the file version number (look for UNSIGNED_TX_PREFIX and SIGNED_TX_PREFIX) |
098337a
to
304660a
Compare
Thanks, I was completely overlooking it. |
Closing for now, as it turns out that Boost's unofficial portable serializer has a flaw, see Issue #1511 |
Reopened due to the cause of the error being understood, see PR #1515 |
Would it make sense to squash this one and #1515 together as a single working PR? |
@NanoAkron That makes sense, thanks for the suggestion! Closing this PR as such. |
As discussed in Issue #2 of onion-monero-blockchain-explorer, currently the serialized files of unsigned/signed transactions are unportable; e.g., the tx pusher hosted by MoneroWorld.com can work accept files created in Linux. This PR solves the problem.