Skip to content

fix: reduce repetitive code in enum match blocks #6453

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.

Sign up for GitHub

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

Merged
hanabi1224 merged 5 commits into from
Jan 23, 2026
Merged

fix: reduce repetitive code in enum match blocks #6453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49a6a88
fix: reduce repetitive code in enum match blocks
hanabi1224 Jan 20, 2026
bd4fddc
Merge branch 'main' into hm/reduce-repetitive-code
hanabi1224 Jan 21, 2026
1882931
more refactoring
hanabi1224 Jan 21, 2026
ea9a27d
Merge remote-tracking branch 'origin/main' into hm/reduce-repetitive-…
hanabi1224 Jan 21, 2026
f060b73
Merge remote-tracking branch 'origin/main' into hm/reduce-repetitive-…
hanabi1224 Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 126 additions & 6 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ similar = "2"
slotmap = "1"
smallvec = "1"
smart-default = "0.7"
spire_enum = "1"
stacker = "0.1"
static_assertions = "1"
statrs = "0.18"
Expand Down
2 changes: 1 addition & 1 deletion src/interpreter/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,5 @@ where

let acc_st = account::State::load(store, act.code, act.state)?;

Ok(acc_st.pubkey_address().into())
Ok(acc_st.pubkey_address())
}
8 changes: 3 additions & 5 deletions src/interpreter/vm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ use fvm4::{
},
};
use num::Zero;
use spire_enum::prelude::delegated_enum;
use std::time::{Duration, Instant};

pub(in crate::interpreter) type ForestMachineV2<DB> =
Expand Down Expand Up @@ -139,6 +140,7 @@ impl BlockMessages {

/// Interpreter which handles execution of state transitioning messages and
/// returns receipts from the VM execution.
#[delegated_enum(impl_conversions)]
pub enum VM<DB: Blockstore + Send + Sync + 'static> {
VM2(ForestExecutorV2<DB>),
VM3(ForestExecutorV3<DB>),
Expand Down Expand Up @@ -272,11 +274,7 @@ where

/// Flush stores in VM and return state root.
pub fn flush(&mut self) -> anyhow::Result<Cid> {
match self {
VM::VM2(fvm_executor) => Ok(fvm_executor.flush()?),
VM::VM3(fvm_executor) => Ok(fvm_executor.flush()?),
VM::VM4(fvm_executor) => Ok(fvm_executor.flush()?),
}
Ok(delegate_vm!(self.flush()?))
Copy link
Copy Markdown
Member

@LesnyRumcajs LesnyRumcajs Jan 21, 2026

Choose a reason for hiding this comment

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

to my understanding, we can avoid the declarative macro via #[delegate_impl], no?

Copy link
Copy Markdown
Contributor Author

@hanabi1224 hanabi1224 Jan 21, 2026

Choose a reason for hiding this comment

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

Tried #[delegate_impl] but it does not compile. According to the doc, #[delegate_impl] is used for trait impls.
https://docs.rs/spire_enum/1.0.0/spire_enum/index.html#2-delegate_impl-inherenttrait-impl-attribute-macro

Copy link
Copy Markdown
Member

@LesnyRumcajs LesnyRumcajs Jan 21, 2026

Choose a reason for hiding this comment

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

Hm, indeed. I wonder if VM should've been a trait. Something to consider in the future, but outside of the scope of this PR.

}

/// Get actor state from an address. Will be resolved to ID address.
Expand Down
2 changes: 1 addition & 1 deletion src/lotus_json/actors/states/account_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl HasLotusJson for State {

fn into_lotus_json(self) -> Self::LotusJson {
AccountStateLotusJson {
address: self.pubkey_address().into(),
address: self.pubkey_address(),
}
}

Expand Down
19 changes: 5 additions & 14 deletions src/shim/actors/builtin/account/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
// SPDX-License-Identifier: Apache-2.0, MIT

use super::super::convert::{from_address_v3_to_v2, from_address_v4_to_v2};
use crate::shim;
use crate::shim::{self, address::Address};
use serde::Serialize;
use spire_enum::prelude::delegated_enum;

/// Account actor method.
pub type Method = fil_actor_account_state::v8::Method;

/// Account actor state.
#[derive(Serialize, Debug)]
#[serde(untagged)]
#[delegated_enum(impl_conversions)]
pub enum State {
V8(fil_actor_account_state::v8::State),
V9(fil_actor_account_state::v9::State),
Expand All @@ -25,19 +27,8 @@ pub enum State {
}

impl State {
pub fn pubkey_address(&self) -> fvm_shared2::address::Address {
match self {
State::V8(st) => st.address,
State::V9(st) => st.address,
State::V10(st) => from_address_v3_to_v2(st.address),
State::V11(st) => from_address_v3_to_v2(st.address),
Copy link
Copy Markdown
Member

@LesnyRumcajs LesnyRumcajs Jan 22, 2026

Choose a reason for hiding this comment

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

How does it do it now? What's the error message in case of a fiasco?

Copy link
Copy Markdown
Contributor Author

@hanabi1224 hanabi1224 Jan 23, 2026

Choose a reason for hiding this comment

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

Sorry, I don't quite get it. This method has no error propogation.

Copy link
Copy Markdown
Member

@LesnyRumcajs LesnyRumcajs Jan 23, 2026

Choose a reason for hiding this comment

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

It doesn't, but it panics with a certain string.

pub fn from_address_v3_to_v2(addr: AddressV3) -> AddressV2 {
    AddressV2::from_bytes(&addr.to_bytes())
        .expect("Couldn't convert between FVM3 and FVM2 addresses.")
}

Given the conversion is not trivial (one has to call from_bytes etc. and check if it worked), my question is what is generated under the hood.

Copy link
Copy Markdown
Contributor Author

@hanabi1224 hanabi1224 Jan 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

No conversion happens here, plz note that the fn return type has been changed to shim::Address. Previously, all addresses are converted to v2, then converted to shim::Address here

Copy link
Copy Markdown
Member

@LesnyRumcajs LesnyRumcajs Jan 23, 2026

Choose a reason for hiding this comment

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

Ah okay, I missed the signature change. LGTM.

State::V12(st) => from_address_v4_to_v2(st.address),
State::V13(st) => from_address_v4_to_v2(st.address),
State::V14(st) => from_address_v4_to_v2(st.address),
State::V15(st) => from_address_v4_to_v2(st.address),
State::V16(st) => from_address_v4_to_v2(st.address),
State::V17(st) => from_address_v4_to_v2(st.address),
}
pub fn pubkey_address(&self) -> Address {
delegate_state!(self.address.into())
}

pub fn default_latest_version(address: fvm_shared4::address::Address) -> Self {
Expand Down
Loading
Loading