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

Refactor: utils::buffer #1247

Open
rrybarczyk opened this issue Nov 13, 2024 · 4 comments
Open

Refactor: utils::buffer #1247

rrybarczyk opened this issue Nov 13, 2024 · 4 comments
Assignees
Labels
refactor Implies refactoring code util Common utilities/helpers

Comments

@rrybarczyk
Copy link
Collaborator

Background

This task is an outcome of the protocols Rust docs issues tracked in #845.

While documenting utils::buffer in #1184, areas of potential code debt were identified. This issue servers as a place to list out these items to then be addressed in an organized manner. The initial Rust documentation effort was an immediate action, while a refactoring (which implies breaking API changes) is not so urgent priority, so for now we should leave this in the backlog for an appropriate moment in the future.

@rrybarczyk rrybarczyk added refactor Implies refactoring code util Common utilities/helpers labels Nov 13, 2024
@rrybarczyk rrybarczyk self-assigned this Nov 13, 2024
@rrybarczyk rrybarczyk moved this to In Progress 🏗️ in SV2 Roadmap 🛣️ Nov 13, 2024
@rrybarczyk
Copy link
Collaborator Author

Questions:

  • Why is buffer_sv2 imported in the roles/pool, /jd-server, /translator, and
    test-utils/mining-device if not used?
  • slice.rs impl Serialize for Slice: why is this implemented if it simply panics when called? is
    it for feature flag compatibility, or some way to indicate that the serialization of Slice is not
    supported, or something else?
  • slice.rs: Why is IGNORE_INDEX set to 59?
  • slice.rs, SharedState::toogle debug mode: What is the mode? what are all the possible modes?
    Just reading or writing? Processing? Other?
  • pool_back.rs try_clear_head, why do we not return a BufferFromSystemMemory and the
    Err(PoolMode::Alloc) like we do in the PoolFront if-else stmt block?

@rrybarczyk
Copy link
Collaborator Author

Suggestions:

  • Rename SharedState::toogle() to toggle.
  • slice.rs: Import core::ops::Index and core::ops::IndexMut directly at top of module.
  • lib.rs: Rename Buffer::get_data_by_ref -> Buffer::get_data_by_ref_mut and
    Buffer::get_data_by_ref_ -> Buffer::get_data_by_ref
  • buffer.rs: Handing tests towards the end of the file, followed by not-test logic. Needs to be
    moved to the bottom of the file inside a mod tests block.

@rrybarczyk
Copy link
Collaborator Author

Why is AEADBuffer handled in this way:

lib.rs exports aes_gcm::aead::Buffer as AeadBuffer. This comes from
here.

@Fi3
Copy link
Collaborator

Fi3 commented Nov 14, 2024

Why is AEADBuffer handled in this way:

lib.rs exports aes_gcm::aead::Buffer as AeadBuffer. This comes from here.

buffer pool is used also for encrypt and decrypt and to do that in place we need that trait

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Implies refactoring code util Common utilities/helpers
Projects
Status: In Progress 🏗️
Development

No branches or pull requests

2 participants