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

Commit

Permalink
bug: issue committing checkpoint source with robust address (#118)
Browse files Browse the repository at this point in the history
* refactor subnet ID

* make clippy happy

* protect against the case where children is empty

* compute chainID from subnetID

* compute chainID from subnetID

* add new rootnet id fn

* fix MAX_CHAIN_ID

* fix unit tests

* support robust address for SubnetID (#113)

* support robust address for SubnetID

* enforce the use of f0-based subnetID in actor

* minor fix in f2 address handling

* minor fix

* Update sdk/src/subnet_id.rs

---------

Signed-off-by: Alfonso de la Rocha <adlrocha@tutamail.com>
  • Loading branch information
adlrocha authored Jun 12, 2023
1 parent c995cd9 commit 4def42c
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 22 deletions.
8 changes: 8 additions & 0 deletions gateway/src/checkpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ impl BottomUpCheckpoint {
}
}

/// Agents may set the source of a checkpoint using f2-based subnetIDs, \
/// but actors are expected to use f0-based subnetIDs, thus the need to enforce
/// that the source is a f0-based subnetID.
pub fn enforce_f0_source(&mut self, rt: &mut impl Runtime) -> anyhow::Result<()> {
self.data.source = self.source().f0_id(rt);
Ok(())
}

/// Get the sum of values in cross messages
pub fn total_value(&self) -> TokenAmount {
match &self.data.cross_msgs.cross_msgs {
Expand Down
18 changes: 9 additions & 9 deletions gateway/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl Actor {
let mut shid = SubnetID::default();
rt.transaction(|st: &mut State, rt| {
shid = SubnetID::new_from_parent(&st.network_name, subnet_addr);
let sub = st.get_subnet(rt.store(), &shid).map_err(|e| {
let sub = st.get_subnet(rt, &shid).map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to load subnet")
})?;
match sub {
Expand Down Expand Up @@ -140,7 +140,7 @@ impl Actor {

rt.transaction(|st: &mut State, rt| {
let shid = SubnetID::new_from_parent(&st.network_name, subnet_addr);
let sub = st.get_subnet(rt.store(), &shid).map_err(|e| {
let sub = st.get_subnet(rt, &shid).map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to load subnet")
})?;
match sub {
Expand Down Expand Up @@ -184,7 +184,7 @@ impl Actor {

rt.transaction(|st: &mut State, rt| {
let shid = SubnetID::new_from_parent(&st.network_name, subnet_addr);
let sub = st.get_subnet(rt.store(), &shid).map_err(|e| {
let sub = st.get_subnet(rt, &shid).map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to load subnet")
})?;
match sub {
Expand Down Expand Up @@ -235,7 +235,7 @@ impl Actor {

rt.transaction(|st: &mut State, rt| {
let shid = SubnetID::new_from_parent(&st.network_name, subnet_addr);
let sub = st.get_subnet(rt.store(), &shid).map_err(|e| {
let sub = st.get_subnet(rt, &shid).map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to load subnet")
})?;
match sub {
Expand Down Expand Up @@ -303,7 +303,7 @@ impl Actor {
st.require_initialized()?;

let shid = SubnetID::new_from_parent(&st.network_name, subnet_addr);
let sub = st.get_subnet(rt.store(), &shid).map_err(|e| {
let sub = st.get_subnet(rt, &shid).map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to load subnet")
})?;

Expand Down Expand Up @@ -436,7 +436,7 @@ impl Actor {
log::debug!("fund cross msg is: {:?}", f_msg);

// Commit top-down message.
st.commit_topdown_msg(rt.store(), &mut f_msg).map_err(|e| {
st.commit_topdown_msg(rt, &mut f_msg).map_err(|e| {
e.downcast_default(
ExitCode::USR_ILLEGAL_STATE,
"error committing top-down message",
Expand Down Expand Up @@ -838,7 +838,7 @@ impl Actor {
// then we need to start propagating it down to the destination.
let r = if nearest_common_parent == st.network_name {
top_down_fee = fee;
st.commit_topdown_msg(rt.store(), cross_msg)
st.commit_topdown_msg(rt, cross_msg)
} else {
if cross_msg.msg.value > TokenAmount::zero() {
do_burn = true;
Expand All @@ -857,7 +857,7 @@ impl Actor {
}
IPCMsgType::TopDown => {
st.applied_topdown_nonce += 1;
st.commit_topdown_msg(rt.store(), cross_msg).map_err(|e| {
st.commit_topdown_msg(rt, cross_msg).map_err(|e| {
e.downcast_default(
ExitCode::USR_ILLEGAL_STATE,
"error committing top-down message while applying it",
Expand Down Expand Up @@ -922,7 +922,7 @@ impl Actor {
if sto == st.network_name {
rt.transaction(|st: &mut State, rt| {
// get applied bottom-up nonce from subnet
match st.get_subnet(rt.store(), forwarder)
match st.get_subnet(rt, forwarder)
.map_err(|_| actor_error!(illegal_argument, "error getting subnet from store in bottom-up execution"))?{
Some(mut sub) => {
if sub.applied_bottomup_nonce != cross_msg.msg.nonce {
Expand Down
18 changes: 9 additions & 9 deletions gateway/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,13 @@ impl State {
}

/// Get content for a child subnet as mut.
pub fn get_subnet<BS: Blockstore>(
pub fn get_subnet(
&mut self,
store: &BS,
rt: &impl Runtime,
id: &SubnetID,
) -> anyhow::Result<Option<Subnet>> {
let subnets = self.subnets.load(store)?;
let subnet = get_subnet(&subnets, id)?;
let subnets = self.subnets.load(rt.store())?;
let subnet = get_subnet(&subnets, &id.f0_id(rt))?;
Ok(subnet.cloned())
}

Expand Down Expand Up @@ -251,17 +251,17 @@ impl State {
}

/// commit topdown messages for their execution in the subnet
pub(crate) fn commit_topdown_msg<BS: Blockstore>(
pub(crate) fn commit_topdown_msg(
&mut self,
store: &BS,
rt: &impl Runtime,
cross_msg: &mut CrossMsg,
) -> anyhow::Result<()> {
let msg = &cross_msg.msg;
let sto = msg.to.subnet()?;

let sub = self
.get_subnet(
store,
rt,
match &sto.down(&self.network_name) {
Some(sub) => sub,
None => return Err(anyhow!("couldn't compute the next subnet in route")),
Expand All @@ -273,10 +273,10 @@ impl State {
match sub {
Some(mut sub) => {
cross_msg.msg.nonce = sub.topdown_nonce;
sub.store_topdown_msg(store, cross_msg)?;
sub.store_topdown_msg(rt.store(), cross_msg)?;
sub.topdown_nonce += 1;
sub.circ_supply += &cross_msg.msg.value;
self.flush_subnet(store, &sub)?;
self.flush_subnet(rt.store(), &sub)?;
}
None => {
return Err(anyhow!(
Expand Down
18 changes: 18 additions & 0 deletions sdk/src/subnet_id.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use fil_actors_runtime::cbor;
use fil_actors_runtime::runtime::Runtime;
use fnv::FnvHasher;
use fvm_shared::address::Address;
use lazy_static::lazy_static;
Expand Down Expand Up @@ -49,6 +50,23 @@ impl SubnetID {
}
}

/// Ensures that the SubnetID only uses f0 addresses for the subnet actor
/// hosted in the current network. The rest of the route is left
/// as-is. We only have information to translate from f2 to f0 for the
/// last subnet actor in the root.
pub fn f0_id(&self, rt: &impl Runtime) -> SubnetID {
let mut children = self.children();

// replace the resolved child (if any)
if let Some(actor_addr) = children.last_mut() {
if let Some(f0) = rt.resolve_address(actor_addr) {
*actor_addr = f0;
}
}

SubnetID::new(self.root_id(), children)
}

/// Creates a new rootnet SubnetID
pub fn new_root(root_id: u64) -> Self {
Self {
Expand Down
2 changes: 1 addition & 1 deletion subnet-actor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl SubnetActor for Actor {
// useful once we support the docking of subnets to new parents, etc.
// let genesis_epoch = rt.curr_epoch();
let genesis_epoch = 0;
let st = State::new(rt.store(), params, genesis_epoch).map_err(|e| {
let st = State::new(rt, params, genesis_epoch).map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "Failed to create actor state")
})?;

Expand Down
7 changes: 4 additions & 3 deletions subnet-actor/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,12 @@ pub struct State {
/// and have load and save methods automatically generated for them as part of a
/// StateObject trait (i.e. impl StateObject for State).
impl State {
pub fn new<BS: Blockstore>(
store: &BS,
pub fn new(
rt: &mut impl Runtime,
params: ConstructParams,
current_epoch: ChainEpoch,
) -> anyhow::Result<State> {
let store = rt.store();
let min_stake = TokenAmount::from_atto(MIN_COLLATERAL_AMOUNT);
let bottomup_check_period = if params.bottomup_check_period < DEFAULT_CHECKPOINT_PERIOD {
DEFAULT_CHECKPOINT_PERIOD
Expand All @@ -82,7 +83,7 @@ impl State {
};
let state = State {
name: params.name,
parent_id: params.parent,
parent_id: params.parent.f0_id(rt),
ipc_gateway_addr: params.ipc_gateway_addr,
consensus: params.consensus,
total_stake: TokenAmount::zero(),
Expand Down

0 comments on commit 4def42c

Please sign in to comment.