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

Fix prefund #405

Closed
wants to merge 1 commit into from
Closed

Fix prefund #405

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 15 additions & 3 deletions ipc/cli/src/commands/subnet/join.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// SPDX-License-Identifier: MIT
//! Join subnet cli command handler.

use anyhow::anyhow;
use async_trait::async_trait;
use clap::Args;
use ipc_sdk::subnet_id::SubnetID;
use num_traits::Zero;
use std::{fmt::Debug, str::FromStr};

use crate::{
Expand All @@ -31,9 +33,19 @@ impl CommandLineHandler for JoinSubnet {
let public_key = hex::decode(&arguments.public_key)?;
if let Some(initial_balance) = arguments.initial_balance {
log::info!("pre-funding address with {initial_balance}");
provider
.pre_fund(subnet.clone(), from, f64_to_token_amount(initial_balance)?)
.await?;
if initial_balance.is_zero() {
return Err(anyhow!("not enough balance"));
Copy link
Contributor

Choose a reason for hiding this comment

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

This check is already being done in the contract, so this one is redundant (in case you want to remove this check): https://github.com/consensus-shipyard/ipc-solidity-actors/blob/9c85f9d110ba10737d258094085e70bb9f0ad54d/src/subnet/SubnetActorManagerFacet.sol#L97

}
let amount = f64_to_token_amount(initial_balance)?;
if let Err(e) = provider
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this fully solves the issues that you linked:

  • For the atomicity of prefund + join. It may be the case that a join has failed but the prefund transaction went through, which means that when calling the command again, additional initial-balance will be passed when the user actually wanted the original initial-balance. The right way of addressing this should be to:
    • Check if the validator already has an initial balance in the contract.
    • If no, then execute pre-fund normally and then call join.
    • If yes, then check if the initial balance is equal to the amount passed in --intial-balance. If it is the same, do not call pre-fund and call join. If the initial balance is not equal to --initial-balance then call pre-fund for the difference in initial balance requested for the user.
  • On the other hand, for the pre-fund/fund difference, we can simplify the logic by:
    • If the subnet is already bootstrapped, calling first join, and then calling fund. That way, if join failed, fund will never be called and we won't have the atomicity problem that we had above.

.pre_fund(subnet.clone(), from, amount.clone())
.await
{
log::info!("pre fund failed with: {}, attend to fund to self", e);
provider
.fund(subnet.clone(), None, from, from, amount)
.await?;
}
}
let epoch = provider
.join_subnet(
Expand Down
Loading