Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

removed unused code #3

Merged
merged 5 commits into from
Oct 17, 2022
Merged

removed unused code #3

merged 5 commits into from
Oct 17, 2022

Conversation

cryptoAtwill
Copy link
Collaborator

Changes

  • Removed unused code such as randomness, runtime policy.
  • Migrated tcid to primitives and added TAddress crate
  • Added default exports in src/lib.rs.
  • Add readme.md to all crates.

Tests

cargo build && cargo test

@@ -12,7 +12,6 @@ use fvm_shared::crypto::signature::Signature;
use fvm_shared::econ::TokenAmount;
use fvm_shared::error::{ErrorNumber, ExitCode};
use fvm_shared::piece::PieceInfo;
use fvm_shared::randomness::Randomness;
use fvm_shared::sector::{
Copy link
Contributor

Choose a reason for hiding this comment

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

All the types that implement sector or storage-specific functions can also be removed.

/// Runtime is the VM's internal runtime object.
/// this is everything that is accessible to actors, beyond parameters.
pub trait Runtime<BS: Blockstore>: Primitives + Verifier + RuntimePolicy {
pub trait Runtime<BS: Blockstore>: Primitives + Verifier {
Copy link
Contributor

Choose a reason for hiding this comment

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

Verifier can also be removed.

@@ -21,7 +21,6 @@ use fvm_shared::crypto::signature::Signature;
use fvm_shared::econ::TokenAmount;
use fvm_shared::error::ExitCode;
use fvm_shared::piece::PieceInfo;
use fvm_shared::randomness::Randomness;
use fvm_shared::sector::{
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, these can probably be removed.

entropy,
out,
};
pub fn expect_get_randomness_from_beacon(&mut self) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to keep these? It is imposed by the trait definition? If this is the case, which one? I feel we can remove this from the runtime.

Copy link
Contributor

@adlrocha adlrocha left a comment

Choose a reason for hiding this comment

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

LGTM.

A few things before you merge:

  • Consider adding also a brief description of what is included as part of the fvm_utils in the main README of the repo.
  • All storage-specific code (like Verifier and sector-related features) can also be removed.
  • Consider removing functions like expect_randomness from the runtime trait definition if needed.

README.md Outdated Show resolved Hide resolved
This repo contains a series of crates that serve as utils for fvm develop and testing.
Here are some simple breakdowns:
- runtime: Contains the runtime wrapper for communicating with `fvm`. It provides some
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: Add an extended description of the runtime. Do you mind adding an issue to remind ourselves to do it in the future (I can probably take this one)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cryptoAtwill and others added 2 commits October 17, 2022 23:22
Co-authored-by: adlrocha <adlrocha@tutamail.com>
@cryptoAtwill cryptoAtwill merged commit ea0a68c into main Oct 17, 2022
@cryptoAtwill cryptoAtwill deleted the patch branch October 17, 2022 15:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants