[seraphis_wallet]: encrypted file #14
Conversation
| decrypted_data.resize(file.encrypted_data.size()); | ||
| crypto::chacha20(file.encrypted_data.data(), file.encrypted_data.size(), key, file.iv, &decrypted_data[0]); | ||
|
|
||
| binary_archive<false> ar{epee::strspan<std::uint8_t>(decrypted_data)}; |
There was a problem hiding this comment.
I would consider using the epee binary output. It is more flexible when adding new fields than this cryptonote scheme (which is more compact).
There was a problem hiding this comment.
using the epee binary output
Which class is this exactly?
The whole wallet2 code is pretty much standardized on binary_archive now, especially since moneromooo deprecated the use of the outdated Boost serialization a few years back. I think binary_archive has proven itself quite well over all that time, and would hesitate to give up the standardization.
There was a problem hiding this comment.
Which class is this exactly?
The whole wallet2 code is pretty much standardized on binary_archive now
I forgot about that. The format is more compact, but you have to remember to add a version field manually in most cases (or some type field), as the field ordering is strict and implied.
The epee binary format is close to msgpack in terms of performance and flexibility (especially when adding new fields).
There was a problem hiding this comment.
Let us stay with the same stuff that wallet2 also uses. Using the same serialization system for the file as a whole as for individual classes seems ok to me for the time being.
There was a problem hiding this comment.
A general question for the PR as a whole: As this will probably be merged first, and be the first file in the not-yet-existing /src/seraphis_wallet folder, doesn't this need some CMakeList.txt file as well to be self-contained, create the folder, and start some library?
Maybe it will take some time to get the key container merge-ready, and therefore a good idea to have a state where other people could already add more code.
I preferred to let this be in the key container PR because it would add a (mostly) empty file, although, the key container seems to still be a bit raw from the comments so maybe |
ghostway0
force-pushed
the
ghostway/encrypted_file
branch
from
62b29e0
to
5378c45
Compare
So be it. I still would have preferred this to be nicely self-contained by adding a CMakeList.txt file, but the code in this PR waited already long enough. We have to get things moving. |
@rbrunner7