Skip to content
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.

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

propagate all gas outputs in ApplyRet #526

Merged
merged 2 commits into from
May 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 17 additions & 6 deletions fvm/src/executor/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,14 @@ where
}),
ApplyKind::Implicit => Ok(ApplyRet {
msg_receipt: receipt,
failure_info,
penalty: TokenAmount::zero(),
miner_tip: TokenAmount::zero(),
base_fee_burn: TokenAmount::from(0),
over_estimation_burn: TokenAmount::from(0),
refund: TokenAmount::from(0),
gas_refund: 0,
gas_burned: 0,
failure_info,
exec_trace,
}),
}
Expand Down Expand Up @@ -326,11 +331,12 @@ where
// NOTE: we don't support old network versions in the FVM, so we always burn.
let GasOutputs {
base_fee_burn,
miner_tip,
over_estimation_burn,
refund,
miner_penalty,
..
miner_tip,
refund,
gas_refund,
gas_burned,
} = GasOutputs::compute(
receipt.gas_used,
msg.gas_limit,
Expand Down Expand Up @@ -365,15 +371,20 @@ where
// refund unused gas
transfer_to_actor(&msg.from, &refund)?;

if (&base_fee_burn + over_estimation_burn + &refund + &miner_tip) != gas_cost {
if (&base_fee_burn + &over_estimation_burn + &refund + &miner_tip) != gas_cost {
// Sanity check. This could be a fatal error.
return Err(anyhow!("Gas handling math is wrong"));
}
Ok(ApplyRet {
msg_receipt: receipt,
failure_info,
penalty: miner_penalty,
miner_tip,
base_fee_burn,
over_estimation_burn,
refund,
gas_refund,
gas_burned,
failure_info,
exec_trace: vec![],
})
}
Expand Down
16 changes: 15 additions & 1 deletion fvm/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::fmt::Display;
pub use default::DefaultExecutor;
use fvm_ipld_encoding::RawBytes;
use fvm_shared::bigint::{BigInt, Sign};
use fvm_shared::econ::TokenAmount;
use fvm_shared::error::ExitCode;
use fvm_shared::message::Message;
use fvm_shared::receipt::Receipt;
Expand Down Expand Up @@ -70,6 +71,14 @@ pub struct ApplyRet {
pub penalty: BigInt,
/// Tip given to miner from message.
pub miner_tip: BigInt,

// Gas stuffs
pub base_fee_burn: TokenAmount,
pub over_estimation_burn: TokenAmount,
pub refund: TokenAmount,
Comment on lines +76 to +78
Copy link
Member

Choose a reason for hiding this comment

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

Let's continue using BigInt here. TokenAmount is aliased to BigInt, so not a big deal, but I'd prefer to contain entropy.

Copy link
Member

Choose a reason for hiding this comment

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

Hm. We should probably go the other way so we can eventually get rid of that alias, right?

pub gas_refund: i64,
pub gas_burned: i64,

/// Additional failure information for debugging, if any.
pub failure_info: Option<ApplyFailure>,
/// Execution trace information, for debugging.
Expand All @@ -90,8 +99,13 @@ impl ApplyRet {
gas_used: 0,
},
penalty: miner_penalty,
failure_info: Some(ApplyFailure::PreValidation(message.into())),
miner_tip: BigInt::zero(),
base_fee_burn: TokenAmount::from(0),
over_estimation_burn: TokenAmount::from(0),
refund: TokenAmount::from(0),
gas_refund: 0,
gas_burned: 0,
failure_info: Some(ApplyFailure::PreValidation(message.into())),
exec_trace: vec![],
}
}
Expand Down
4 changes: 2 additions & 2 deletions fvm/src/system_actor.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
use anyhow::Context;
use cid::Cid;
use fvm_ipld_blockstore::Blockstore;
use fvm_ipld_encoding::tuple::{Deserialize_tuple, Serialize_tuple};
use fvm_ipld_encoding::{Cbor, CborStore};
use fvm_shared::address::Address;
use serde::{Deserialize, Serialize};

use crate::kernel::{ClassifyResult, Result};
use crate::state_tree::{ActorState, StateTree};

pub const SYSTEM_ACTOR_ADDR: Address = Address::new_id(0);

#[derive(Default, Deserialize, Serialize)]
#[derive(Default, Deserialize_tuple, Serialize_tuple)]
pub struct State {
// builtin actor registry: Vec<(String, Cid)>
pub builtin_actors: Cid,
Expand Down