From 5be125ad1afe01ef6f3eff58eca9091623571bf5 Mon Sep 17 00:00:00 2001 From: Aayush Date: Wed, 16 Feb 2022 23:21:06 -0500 Subject: [PATCH] address review feedback --- chain/vm/fvm.go | 68 +++++++++++++++++++------------------------------ 1 file changed, 26 insertions(+), 42 deletions(-) diff --git a/chain/vm/fvm.go b/chain/vm/fvm.go index f98c043dd..962b27be6 100644 --- a/chain/vm/fvm.go +++ b/chain/vm/fvm.go @@ -4,6 +4,8 @@ import ( "bytes" "context" + "github.com/filecoin-project/go-state-types/big" + "github.com/filecoin-project/lotus/chain/state" cbor "github.com/ipfs/go-ipld-cbor" @@ -36,13 +38,12 @@ type FvmExtern struct { base cid.Cid } -// Similar to the one in syscalls.go used by the Lotus VM, except it never errors +// VerifyConsensusFault is similar to the one in syscalls.go used by the Lotus VM, except it never errors // Errors are logged and "no fault" is returned, which is functionally what go-actors does anyway -func (x *FvmExtern) VerifyConsensusFault(ctx context.Context, a, b, extra []byte) *ffi_cgo.ConsensusFaultWithGas { - ret := &ffi_cgo.ConsensusFaultWithGas{ - // TODO: is this gonna be a problem on the receiving end? should we return address.NewIDAddress(0) instead? - Target: address.Undef, - Type: ffi_cgo.ConsensusFaultNone, +func (x *FvmExtern) VerifyConsensusFault(ctx context.Context, a, b, extra []byte) (*ffi_cgo.ConsensusFault, int64) { + totalGas := int64(0) + ret := &ffi_cgo.ConsensusFault{ + Type: ffi_cgo.ConsensusFaultNone, } // Note that block syntax is not validated. Any validly signed block will be accepted pursuant to the below conditions. @@ -56,42 +57,31 @@ func (x *FvmExtern) VerifyConsensusFault(ctx context.Context, a, b, extra []byte var blockA, blockB types.BlockHeader if decodeErr := blockA.UnmarshalCBOR(bytes.NewReader(a)); decodeErr != nil { log.Info("invalid consensus fault: cannot decode first block header: %w", decodeErr) - return ret + return ret, totalGas } if decodeErr := blockB.UnmarshalCBOR(bytes.NewReader(b)); decodeErr != nil { log.Info("invalid consensus fault: cannot decode second block header: %w", decodeErr) - return ret + return ret, totalGas } - // Commented out from the Lotus VM code: FvmExtern only supports v14 and onwards - // workaround chain halt - //if build.IsNearUpgrade(blockA.Height, build.UpgradeOrangeHeight) { - // return nil, xerrors.Errorf("consensus reporting disabled around Upgrade Orange") - //} - //if build.IsNearUpgrade(blockB.Height, build.UpgradeOrangeHeight) { - // return nil, xerrors.Errorf("consensus reporting disabled around Upgrade Orange") - //} - // are blocks the same? if blockA.Cid().Equals(blockB.Cid()) { log.Info("invalid consensus fault: submitted blocks are the same") - return ret + return ret, totalGas } // (1) check conditions necessary to any consensus fault // were blocks mined by same miner? if blockA.Miner != blockB.Miner { log.Info("invalid consensus fault: blocks not mined by the same miner") - return ret + return ret, totalGas } - ret.Target = blockA.Miner - // block a must be earlier or equal to block b, epoch wise (ie at least as early in the chain). if blockB.Height < blockA.Height { log.Info("invalid consensus fault: first block must not be of higher height than second") - return ret + return ret, totalGas } ret.Epoch = blockB.Height @@ -123,9 +113,7 @@ func (x *FvmExtern) VerifyConsensusFault(ctx context.Context, a, b, extra []byte if len(extra) > 0 { if decodeErr := blockC.UnmarshalCBOR(bytes.NewReader(extra)); decodeErr != nil { log.Info("invalid consensus fault: cannot decode extra: %w", decodeErr) - // just to match Lotus VM consensus, zero out any already-set faults - faultType = ffi_cgo.ConsensusFaultNone - return ret + return ret, totalGas } if types.CidArrsEqual(blockA.Parents, blockC.Parents) && blockA.Height == blockC.Height && @@ -137,7 +125,7 @@ func (x *FvmExtern) VerifyConsensusFault(ctx context.Context, a, b, extra []byte // (3) return if no consensus fault by now if faultType == ffi_cgo.ConsensusFaultNone { log.Info("invalid consensus fault: no fault detected") - return ret + return ret, totalGas } // else @@ -146,23 +134,24 @@ func (x *FvmExtern) VerifyConsensusFault(ctx context.Context, a, b, extra []byte // check blocks are properly signed by their respective miner // note we do not need to check extra's: it is a parent to block b // which itself is signed, so it was willingly included by the miner - gasUsed, sigErr := x.VerifyBlockSig(ctx, &blockA) - ret.GasUsed += gasUsed + gasA, sigErr := x.VerifyBlockSig(ctx, &blockA) + totalGas += gasA if sigErr != nil { log.Info("invalid consensus fault: cannot verify first block sig: %w", sigErr) - return ret + return ret, totalGas } - gasUsed, sigErr = x.VerifyBlockSig(ctx, &blockB) - ret.GasUsed += gasUsed + gas2, sigErr := x.VerifyBlockSig(ctx, &blockB) + totalGas += gas2 if sigErr != nil { log.Info("invalid consensus fault: cannot verify second block sig: %w", sigErr) - return ret + return ret, totalGas } ret.Type = faultType - - return ret + ret.Target = blockA.Miner + + return ret, totalGas } func (x *FvmExtern) VerifyBlockSig(ctx context.Context, blk *types.BlockHeader) (int64, error) { @@ -175,11 +164,6 @@ func (x *FvmExtern) VerifyBlockSig(ctx context.Context, blk *types.BlockHeader) } func (x *FvmExtern) workerKeyAtLookback(ctx context.Context, minerId address.Address, height abi.ChainEpoch) (address.Address, int64, error) { - // Commented out from the Lotus VM code: FvmExtern only supports v14 and onwards - //if x.networkVersion >= network.Version7 && height < x.epoch-policy.ChainFinality { - // return address.Undef, xerrors.Errorf("cannot get worker key (currEpoch %d, height %d)", ss.epoch, height) - //} - gasUsed := int64(0) gasAdder := func(gc GasCharge) { // technically not overflow safe, but that's fine @@ -261,11 +245,11 @@ func (vm *FVM) ApplyMessage(ctx context.Context, cmsg types.ChainMsg) (*ApplyRet }, GasCosts: &GasOutputs{ // TODO: do the other optional fields eventually - BaseFeeBurn: abi.TokenAmount{}, - OverEstimationBurn: abi.TokenAmount{}, + BaseFeeBurn: big.Zero(), + OverEstimationBurn: big.Zero(), MinerPenalty: ret.MinerPenalty, MinerTip: ret.MinerTip, - Refund: abi.TokenAmount{}, + Refund: big.Zero(), GasRefund: 0, GasBurned: 0, },