Skip to content

Conversation

@michaeldjeffrey
Copy link
Contributor

file-store Refactor: Separate Helium Proto Concerns

This refactor extracts helium-proto specific functionality from file-store into a new crate: file-store-helium-proto. The goal is to remove helium-proto as a dependency from file-store. file-store remains protobuf-specific (using prost), but now developers can use file-store with their own local protos without needing to import helium-proto. All Helium-specific protocol mappings and types are consolidated into file-store-helium-proto.

file-store Cleanup

Remove MsgBytes Trait

The MsgBytes trait has been removed from file-store. Previously, this trait existed to abstract over different message types that could be written to a FileSink. The trait had to be supplementally implemented for any types that wanted to use FileSinkWriteExt. Now FileSinkWriteExt can be used with any protobuf type that implements prost::Message.

Remove helium-proto Dependency

file-store no longer depends on helium-proto. As part of this change:

  • UnknownEnumValue error now comes from prost instead of helium-proto
  • All helium-proto specific mappings have been moved to file-store-helium-proto

General Cleanup

Several unused dependencies and code were removed:

  • Removed csv dependency and related error variant
  • Removed config dependency and ConfigError variant
  • Removed unused error variants: JoinError, Shutdown
  • Improved FileInfoPollerError to include context: {0} in error message
  • Removed unused dependencies: clap, strum, strum_macros, blake3, beacon, humantime-serde

CLI Commands Moved

All CLI commands have been moved to file-store-helium-proto and now return anyhow::Result.

file-store-helium-proto

Structure

The new crate is organized into logical groupings that mirror the Helium network structure:

  • iot/ - IoT-specific protocol mappings (beacon reports, witness reports, packets, valid/invalid PoC)
  • mobile/ - Mobile-specific protocol mappings (coverage, sessions, speedtests, wifi heartbeats, bans, subscriber info, transfers)
  • subscriber/ - Subscriber-specific mappings (verified mapping events, unique connections, usage counts)
  • network_common/ - Cross-network shared types (entropy reports, reward manifests)
  • file_type.rs - FileType enum moved from file-store
  • traits/ - Helium-specific traits

Traits Moved

These traits were specific to helium-proto types have been moved to avoid orphan rules:

  • MsgVerify - for verifying signed messages
  • FileSinkWriteExt - extension trait for writing to file sinks with Helium-specific rollover and commit strategies
  • MsgTimestamp - converting u64 <-> DateTime<Utc> for proto <-> domain struct mappings

The MsgTimestamp trait remains in file-store-helium-proto as traits::MsgTimestamp since it's used across multiple Helium proto types. Moving this trait created orphan rule issues initially, but these have been resolved by implementing the trait within the same crate that defines the types.

FileType Enum

The FileType enum has been moved from file-store to file-store-helium-proto since it's entirely Helium-specific.

CLI

The CLI (file-store-helium-proto binary) provides commands for working with Helium file-store data.
The print_json utility function has also been moved here since it's used by the CLI commands.

Codebase Updates

All downstream crates have been updated to use file-store-helium-proto for Helium-specific types and traits:

  • ingest
  • iot_config
  • iot_packet_verifier
  • iot_verifier
  • mobile_config
  • mobile_packet_verifier
  • mobile_verifier
  • poc_entropy
  • price
  • reward_index
  • solana

Test Updates

In test files, you'll see imports like use file_store_helium_proto::coverage instead of shorter paths. This is intentional - many test files have local modules named coverage, mobile_session, etc. Using the fully qualified file_store_helium_proto:: prefix makes it immediately clear which types are from the library vs. local test helpers.

Import Changes

Here's an example of some common imports that may have changed.

Before:

use file_store::{
    entropy_report::EntropyReport,
    file_info_poller::FileInfoStream,
    file_sink::FileSinkClient,
    traits::{
        FileSinkCommitStrategy, FileSinkRollTime, FileSinkWriteExt,
        MsgTimestamp, TimestampDecode, TimestampEncode,
        MsgDecode, MsgVerify
    },
    DecodeError, Error, Result
};

After:

use file_store::{
    file_info_poller::FileInfoStream,
    file_sink::FileSinkClient,
    traits::{TimestampDecode, TimestampEncode},
    DecodeError, Error, Result
};
use file_store_helium_proto::{
    network_common::entropy_report::EntropyReport,
    traits::{
        FileSinkCommitStrategy, FileSinkRollTime, FileSinkWriteExt,
        MsgTimestamp, MsgVerify,
    },
};

Nothing is reading/writing to csv in file-store. And the dump command 
follows the pattern of dumping json to stdout.
file-store proper doesn’t really do anything with settings (cli 
commands, but okay), to where we need to wrap a config error in a 
special library type.

We can return a config::ConfigError and the caller can map it how they 
need on their application startup.
We don’t need a special library error from a cli. The internals can 
return file_store::Error, and we can turn them into anyhow. This is 
planning for being able to break the cli out into a proto specific 
workspace for helium proto types.
cli dump command to return anyhow::Result

and use MsgDecode where we can and it doesn’t confuse things
- ReportId and IngestId to iot_verifier where they’re used solely
- TimestampEncode/Decode to file_store_shared
…e-helium-proto

This operation is confusing because we run into a slew of rust orphan 
rule problems. We’re trying to keep the type we’re writing in 
FileSink<T>, but cannot tie it to a trait within file-store, because we 
don’t have enough leverage outside of file-store to implement it for 
anything.
This get’s rid of the need for MsgBytes, but keeps us typed over a proto
message that that can serialize itself to bytes.
We can turn into FileInfo from anything that knows how to stringify 
itself and a DateTime.
Breaking out those shared components helped me think through a bunch of 
the changes that were made, but they don’t need to be their workspace 
anymore.
remove file-store-shared
It was nice to have all that out of the way when working on refactors, 
but it’s not useful enough on its own to deserve a workspace.
remove file-store-helium-cli
remove more cli code
The CLI is application layer, and does not need to constrain itself to a
file_store::Error.
doesn’t make sense to remove this one yet imho
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

Successfully merging this pull request may close these issues.

2 participants