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

Contribute StableMemory/icfs back to cdk-rs #25

Open
paulyoung opened this issue Apr 21, 2022 · 4 comments
Open

Contribute StableMemory/icfs back to cdk-rs #25

paulyoung opened this issue Apr 21, 2022 · 4 comments

Comments

@paulyoung
Copy link
Member

paulyoung commented Apr 21, 2022

The core crate here is necessary different to what is found in dfinity/cdk-rs.

The main differences are that there’s a single struct that implements the Read and Write traits, and the Seek trait is also provided.

If there isn’t a significant downside to doing that in the Rust SDK the goal should be to move it (and the tests) back upstream if it would be accepted.

Duplicating efforts here, in cdk-rs, and in other places like https://github.com/spinner-cash/lib-rs/blob/4ba800569bd45dccb3712ba232d5338a2fb8275c/src/storage.rs seems unnecessary.

@paulyoung
Copy link
Member Author

@hpeebles what do you think about this?

@ncpenke
Copy link

ncpenke commented Nov 5, 2022

@paulyoung In case you're interested I have an alternate approach here that layers the functionality on top of the existing cdk-rs API.

I also opened these tickets to potentially merge this work:
dfinity/cdk-rs#333
dfinity/cdk-rs#334

@paulyoung
Copy link
Member Author

@ncpenke thanks for letting me know.

I spent a lot of time testing and fixing edge cases in my implementation so I’d be looking for the same test coverage in an alternative before I’d consider switching.

The examples in this repo double up as test suites. The low-level tests can be found here: https://github.com/codebase-labs/icfs/blob/8371ab72995ea4cd0d786de606a1e6b97a41be0a/examples/icfs/lib.rs

@ncpenke
Copy link

ncpenke commented Nov 7, 2022

Thanks for the pointer @paulyoung. I'll run those tests and let you know the results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants