Change MD5 hash construction to avoid false-positives #140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
July 31, 2021 18:25
Update to quentinsf#100, see quentinsf#134. Note that this will cause current transactions to become unique. Appropriate adjustments have been made to existing tests.
* Added it to `README.md`, made sure it looks alright in the `--help` docs. * Includes a new test for '--depreciated-md5', with a new transaction in transfer.csv. * Added a small informational header to parsed_transfer.txt, which already wasn't the literal (ie. untouched) result of parsing transfer.csv because of the removal of the last transaction. * I think a great alternative would be a seperate script that updates the hashes in a given file (given the appropriate source account) based on either the inline CSV record or the input file that was used. * An implimentation of the `Inline CSV + Source Account` method must check that the existing hash actually matches an old approach to hashes before updating it!
It doesn't look like this is used for anything, and the tests don't fail without it, but correct me if I'm wrong ofc
This was referenced Aug 1, 2021
|
I'll be coming back 'round to (h)ledger later this year, and while I plan on creating my own solution in the long term, I'd be glad to merge my existing PRs and to triage other issues in the meantime |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Re-submission of #139 because I was naively working on master and, as it happens, Github doesn't let you change your default branch, only rename it.
Cleaned up the commits while I was at it.
Note that this will cause current transactions to become unique. It may catch existing users off-guard. Clean solutions welcome.
(I'd like to write an accompanying helper-script that, when given a Source Account, checks an existing ledger for CSV comments accompanied by a hash matching that of a depreciated approach and updates it in-place, but I only have so much time atm.
The justification for breaking it out into a separate script would be that in taking only one file and preforming in-place modifications to it, it would be breaking the conventions of the existing
icsv2ledgercommand.)As it stands, such duplicates may still be detected via
--depreciated-md5, which is disabled by default so as to minimize the ironic 'duplication' of work entailed by checking for every hash every time. You could have this on by default to minimize the aforementioned potential for disruption to existing users.All appropriate adjustments have been made to existing tests.