reorder transfer checks so as to ensure sending more money than you have to yourself fails with an error (fixing issue 7596)

PR #7637, also adds tests to make sure behavior is correct across versions.
This commit is contained in:
c r 2021-11-29 17:26:47 -05:00 committed by GitHub
parent 9d4489360b
commit e16e9ad343
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 174 additions and 28 deletions

View File

@ -890,6 +890,11 @@ workflows:
suite: itest-sector_terminate
target: "./itests/sector_terminate_test.go"
- test:
name: test-itest-self_sent_txn
suite: itest-self_sent_txn
target: "./itests/self_sent_txn_test.go"
- test:
name: test-itest-tape
suite: itest-tape

View File

@ -332,7 +332,7 @@ func (rt *Runtime) DeleteActor(beneficiary address.Address) {
}
// Transfer the executing actor's balance to the beneficiary
if err := rt.vm.transfer(rt.Receiver(), beneficiary, act.Balance); err != nil {
if err := rt.vm.transfer(rt.Receiver(), beneficiary, act.Balance, rt.NetworkVersion()); err != nil {
panic(aerrors.Fatalf("failed to transfer balance to beneficiary actor: %s", err))
}
}

View File

@ -339,7 +339,7 @@ func (vm *VM) send(ctx context.Context, msg *types.Message, parent *Runtime,
defer rt.chargeGasSafe(newGasCharge("OnMethodInvocationDone", 0, 0))
if types.BigCmp(msg.Value, types.NewInt(0)) != 0 {
if err := vm.transfer(msg.From, msg.To, msg.Value); err != nil {
if err := vm.transfer(msg.From, msg.To, msg.Value, vm.ntwkVersion(ctx, vm.blockHeight)); err != nil {
return nil, aerrors.Wrap(err, "failed to transfer funds")
}
}
@ -869,17 +869,55 @@ func (vm *VM) incrementNonce(addr address.Address) error {
})
}
func (vm *VM) transfer(from, to address.Address, amt types.BigInt) aerrors.ActorError {
if from == to {
return nil
func (vm *VM) transfer(from, to address.Address, amt types.BigInt, networkVersion network.Version) aerrors.ActorError {
var f *types.Actor
var fromID, toID address.Address
var err error
// switching the order around so that transactions for more than the balance sent to self fail
if networkVersion >= network.Version15 {
if amt.LessThan(types.NewInt(0)) {
return aerrors.Newf(exitcode.SysErrForbidden, "attempted to transfer negative value: %s", amt)
}
fromID, err := vm.cstate.LookupID(from)
fromID, err = vm.cstate.LookupID(from)
if err != nil {
return aerrors.Fatalf("transfer failed when resolving sender address: %s", err)
}
toID, err := vm.cstate.LookupID(to)
f, err = vm.cstate.GetActor(fromID)
if err != nil {
return aerrors.Fatalf("transfer failed when retrieving sender actor: %s", err)
}
if f.Balance.LessThan(amt) {
return aerrors.Newf(exitcode.SysErrInsufficientFunds, "transfer failed, insufficient balance in sender actor: %v", f.Balance)
}
if from == to {
log.Infow("sending to same address: noop", "from/to addr", from)
return nil
}
toID, err = vm.cstate.LookupID(to)
if err != nil {
return aerrors.Fatalf("transfer failed when resolving receiver address: %s", err)
}
if fromID == toID {
log.Infow("sending to same actor ID: noop", "from/to actor", fromID)
return nil
}
} else {
if from == to {
return nil
}
fromID, err = vm.cstate.LookupID(from)
if err != nil {
return aerrors.Fatalf("transfer failed when resolving sender address: %s", err)
}
toID, err = vm.cstate.LookupID(to)
if err != nil {
return aerrors.Fatalf("transfer failed when resolving receiver address: %s", err)
}
@ -892,27 +930,28 @@ func (vm *VM) transfer(from, to address.Address, amt types.BigInt) aerrors.Actor
return aerrors.Newf(exitcode.SysErrForbidden, "attempted to transfer negative value: %s", amt)
}
f, err := vm.cstate.GetActor(fromID)
f, err = vm.cstate.GetActor(fromID)
if err != nil {
return aerrors.Fatalf("transfer failed when retrieving sender actor: %s", err)
}
}
t, err := vm.cstate.GetActor(toID)
if err != nil {
return aerrors.Fatalf("transfer failed when retrieving receiver actor: %s", err)
}
if err := deductFunds(f, amt); err != nil {
if err = deductFunds(f, amt); err != nil {
return aerrors.Newf(exitcode.SysErrInsufficientFunds, "transfer failed when deducting funds (%s): %s", types.FIL(amt), err)
}
depositFunds(t, amt)
if err := vm.cstate.SetActor(fromID, f); err != nil {
return aerrors.Fatalf("transfer failed when setting receiver actor: %s", err)
if err = vm.cstate.SetActor(fromID, f); err != nil {
return aerrors.Fatalf("transfer failed when setting sender actor: %s", err)
}
if err := vm.cstate.SetActor(toID, t); err != nil {
return aerrors.Fatalf("transfer failed when setting sender actor: %s", err)
if err = vm.cstate.SetActor(toID, t); err != nil {
return aerrors.Fatalf("transfer failed when setting receiver actor: %s", err)
}
return nil

View File

@ -0,0 +1,102 @@
package itests
import (
"context"
"testing"
"time"
"github.com/filecoin-project/go-state-types/network"
"github.com/filecoin-project/go-state-types/big"
"github.com/filecoin-project/go-state-types/exitcode"
"github.com/filecoin-project/lotus/api"
"github.com/filecoin-project/lotus/chain/types"
"github.com/filecoin-project/lotus/itests/kit"
"github.com/stretchr/testify/require"
)
// these tests check that the versioned code in vm.transfer is functioning correctly across versions!
// we reordered the checks to make sure that a transaction with too much money in it sent to yourself will fail instead of succeeding as a noop
// more info in this PR! https://github.com/filecoin-project/lotus/pull/7637
func TestSelfSentTxnV15(t *testing.T) {
ctx := context.Background()
kit.QuietMiningLogs()
client15, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.GenesisNetworkVersion(network.Version15))
ens.InterconnectAll().BeginMining(10 * time.Millisecond)
bal, err := client15.WalletBalance(ctx, client15.DefaultKey.Address)
require.NoError(t, err)
// send self half of account balance
msgHalfBal := &types.Message{
From: client15.DefaultKey.Address,
To: client15.DefaultKey.Address,
Value: big.Div(bal, big.NewInt(2)),
}
smHalfBal, err := client15.MpoolPushMessage(ctx, msgHalfBal, nil)
require.NoError(t, err)
mLookup, err := client15.StateWaitMsg(ctx, smHalfBal.Cid(), 3, api.LookbackNoLimit, true)
require.NoError(t, err)
require.Equal(t, exitcode.Ok, mLookup.Receipt.ExitCode)
msgOverBal := &types.Message{
From: client15.DefaultKey.Address,
To: client15.DefaultKey.Address,
Value: big.Mul(big.NewInt(2), bal),
GasLimit: 10000000000,
GasPremium: big.NewInt(10000000000),
GasFeeCap: big.NewInt(100000000000),
Nonce: 1,
}
smOverBal, err := client15.WalletSignMessage(ctx, client15.DefaultKey.Address, msgOverBal)
require.NoError(t, err)
smcid, err := client15.MpoolPush(ctx, smOverBal)
require.NoError(t, err)
mLookup, err = client15.StateWaitMsg(ctx, smcid, 3, api.LookbackNoLimit, true)
require.NoError(t, err)
require.Equal(t, exitcode.SysErrInsufficientFunds, mLookup.Receipt.ExitCode)
}
func TestSelfSentTxnV14(t *testing.T) {
ctx := context.Background()
kit.QuietMiningLogs()
client14, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.GenesisNetworkVersion(network.Version14))
ens.InterconnectAll().BeginMining(10 * time.Millisecond)
bal, err := client14.WalletBalance(ctx, client14.DefaultKey.Address)
require.NoError(t, err)
// send self half of account balance
msgHalfBal := &types.Message{
From: client14.DefaultKey.Address,
To: client14.DefaultKey.Address,
Value: big.Div(bal, big.NewInt(2)),
}
smHalfBal, err := client14.MpoolPushMessage(ctx, msgHalfBal, nil)
require.NoError(t, err)
mLookup, err := client14.StateWaitMsg(ctx, smHalfBal.Cid(), 3, api.LookbackNoLimit, true)
require.NoError(t, err)
require.Equal(t, exitcode.Ok, mLookup.Receipt.ExitCode)
msgOverBal := &types.Message{
From: client14.DefaultKey.Address,
To: client14.DefaultKey.Address,
Value: big.Mul(big.NewInt(2), bal),
GasLimit: 10000000000,
GasPremium: big.NewInt(10000000000),
GasFeeCap: big.NewInt(100000000000),
Nonce: 1,
}
smOverBal, err := client14.WalletSignMessage(ctx, client14.DefaultKey.Address, msgOverBal)
require.NoError(t, err)
smcid, err := client14.MpoolPush(ctx, smOverBal)
require.NoError(t, err)
mLookup, err = client14.StateWaitMsg(ctx, smcid, 3, api.LookbackNoLimit, true)
require.NoError(t, err)
require.Equal(t, exitcode.Ok, mLookup.Receipt.ExitCode)
}