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

Prepare for npm publish and also reworking the cryptoworker and additional tests #23

Merged
merged 6 commits into from
May 19, 2020

Conversation

robert-cronin
Copy link
Contributor

@robert-cronin robert-cronin commented May 13, 2020

This is to prepare and eventually do npm publish
Fixes #16

@CMCDragonkai
Copy link
Member

I think to go through the crypto code we would need to have a meeting. It would be difficult for me to review it on my own.

@robert-cronin
Copy link
Contributor Author

I think to go through the crypto code we would need to have a meeting. It would be difficult for me to review it on my own.

I agree, there is quite a bit to go through here, let me know when is best 👍

Copy link
Member

@CMCDragonkai CMCDragonkai left a comment

Choose a reason for hiding this comment

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

Main thing is to add as much fuzzing tests to the readSync/writeSync functions because they are really complex.

One way to do this is to check synchronous behaviour "bisimulation" between real fs and EFS when seeking randomly and reading randomly and writing randomly. And comparing the pre and post conditions.

Just a note to write down, page cache for read hasn't been implemented, whereas page cache has been implemented for "write then read". For read, it requires us to figure out the chunk mapping index structure because we need to keep track of "true nulls" vs nulls of convenience ("non-existent nulls").

src/Crypto.ts Outdated Show resolved Hide resolved
src/Crypto.ts Outdated Show resolved Hide resolved
src/Crypto.ts Outdated Show resolved Hide resolved
src/Crypto.ts Outdated Show resolved Hide resolved
src/Crypto.ts Outdated Show resolved Hide resolved
src/Crypto.ts Outdated Show resolved Hide resolved
src/Crypto.ts Outdated Show resolved Hide resolved
src/Crypto.ts Outdated Show resolved Hide resolved
src/CryptoWorker.ts Outdated Show resolved Hide resolved
tests/Cryptor.unit.test.ts Outdated Show resolved Hide resolved
@CMCDragonkai CMCDragonkai changed the title Prepare for npm publish Prepare for npm publish and also reworking the cryptoworker and additional tests May 18, 2020
@robert-cronin
Copy link
Contributor Author

I've added quite a few bisimulation tests that test all the way from 1-3 blocks, 1-2 rounds of read/write as well as bisimulation tests for readFileSync and writeFileSync.

@CMCDragonkai
Copy link
Member

Ok I'm merging this.

@CMCDragonkai CMCDragonkai merged commit 2752577 into master May 19, 2020
@CMCDragonkai CMCDragonkai deleted the npm-release branch May 19, 2020 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development
Development

Successfully merging this pull request may close these issues.

Integrate Generic Crypto library
2 participants