VM gas usage implementation and refactor #350
Conversation
… austin/vm/gasusage
austinabell
requested review from
ansermino,
dutterbutter and
ec2
as code owners
| self.gas_tracker.borrow().gas_used() | ||
| } | ||
|
|
||
| fn gas_available(&self) -> i64 { |
There was a problem hiding this comment.
private method but sure I'll add one
| } | ||
| /// Returns gas required for signature verification | ||
| #[inline] | ||
| pub fn on_verify_signature(&self, sig_type: SignatureType, plain_text_size: usize) -> i64 { |
There was a problem hiding this comment.
I notice in the lotus implementation that they are passing back a SysErrInternal exit code if the retrieval of the costFn fails. Do we need to match this exit code?
Also, I would include the following TODO as mentioned here.
this cost is not persistable, create a LinearCost{a,b} struct that has a .Cost(x) -> ax + b
There was a problem hiding this comment.
nah, not needed, they are using a map to retrieve the function, we are just matching on all signature types, impossible for us to hit this
| _proof: RegisteredProof, | ||
| _pieces: &[PieceInfo], | ||
| ) -> i64 { | ||
| self.compute_unsealed_sector_cid_base |
There was a problem hiding this comment.
There is nothing else TODO for all of these, but I will add a comment to revisit as this probably will change in future
There was a problem hiding this comment.
I figured a TODO must be required since we are not using the params?
| } | ||
| /// Returns gas required for seal verification | ||
| #[inline] | ||
| pub fn on_verify_seal(&self, _info: &SealVerifyInfo) -> i64 { |
| } | ||
| /// Returns gas required for PoSt verification | ||
| #[inline] | ||
| pub fn on_verify_post(&self, _info: &PoStVerifyInfo) -> i64 { |
|
|
||
| /// Returns gas price list by Epoch for gas consumption | ||
| pub fn price_list_by_epoch(_epoch: ChainEpoch) -> PriceList { | ||
| // In future will match on epoch and select matching price lists when config options allowed |
There was a problem hiding this comment.
There is nothing to do here, the protocol is not supporting different price tables yet (unless I am mistaken), this is just a feature
Summary of changes
Changes introduced in this pull request:
Implements gas consumption with explicit calls and implicit through
GasBlockStoreandGasSyscallsVM Gas metering and usage #332Rc<RefCell<>>which can have a runtime panic if a mutable borrow is attempted when any other borrow exists so make sure if theGasTrackeris borrowed that it is dropped as soon as possibleRemoves error from Cid generation from Cbor bytes
Refactored some error handling, downcasting errors in future is going to be needed to match the patterns in go to interop 1:1 with exit codes and this is a step toward that
Reference issue to close (if applicable)
Closes #332
Other information and links