Gas payment change for audit issue M02

core/state_transition.go

@@ -234,44 +234,47 @@ func (st *StateTransition) canPayFee(accountOwner common.Address, fee *big.Int,
 	return balanceOf.Cmp(fee) > 0
 }
 
-func (st *StateTransition) debitFrom(address common.Address, amount *big.Int, feeCurrency *common.Address) error {
+func (st *StateTransition) debitGas(address common.Address, amount *big.Int, feeCurrency *common.Address) error {
 	if amount.Cmp(big.NewInt(0)) == 0 {
 		return nil
 	}
 	evm := st.evm
-	// Function is "debitFrom(address from, uint256 value)"
-	// selector is first 4 bytes of keccak256 of "debitFrom(address,uint256)"
+	// Function is "debitGasFees(address from, uint256 value)"
+	// selector is first 4 bytes of keccak256 of "debitGasFees(address,uint256)"
 	// Source:
 	// pip3 install pyethereum
-	// python3 -c 'from ethereum.utils import sha3; print(sha3("debitFrom(address,uint256)")[0:4].hex())'
-	functionSelector := hexutil.MustDecode("0x362a5f80")
+	// python3 -c 'from ethereum.utils import sha3; print(sha3("debitGasFees(address,uint256)")[0:4].hex())'
+	functionSelector := hexutil.MustDecode("0x58cf9672")
 	transactionData := common.GetEncodedAbi(functionSelector, [][]byte{common.AddressToAbi(address), common.AmountToAbi(amount)})
 
 	rootCaller := vm.AccountRef(common.HexToAddress("0x0"))
 	// The caller was already charged for the cost of this operation via IntrinsicGas.
-	_, leftoverGas, err := evm.Call(rootCaller, *feeCurrency, transactionData, params.MaxGasForDebitFromTransactions, big.NewInt(0))
-	gasUsed := params.MaxGasForDebitFromTransactions - leftoverGas
-	log.Debug("debitFrom called", "feeCurrency", *feeCurrency, "gasUsed", gasUsed)
+	_, leftoverGas, err := evm.Call(rootCaller, *feeCurrency, transactionData, params.MaxGasForDebitGasFeesTransactions, big.NewInt(0))
+	gasUsed := params.MaxGasForDebitGasFeesTransactions - leftoverGas
+	log.Trace("debitGasFees called", "feeCurrency", *feeCurrency, "gasUsed", gasUsed)
 	return err
 }
 
-func (st *StateTransition) creditTo(address common.Address, amount *big.Int, feeCurrency *common.Address) error {
-	if amount.Cmp(big.NewInt(0)) == 0 {
-		return nil
-	}
+func (st *StateTransition) creditGasFees(
+	from common.Address,
+	feeRecipient common.Address,
+	gatewayFeeRecipient *common.Address,
+	communityFund *common.Address,
+	refund *big.Int,
+	tipTxFee *big.Int,
+	gatewayFee *big.Int,
+	baseTxFee *big.Int,
+	feeCurrency *common.Address) error {
 	evm := st.evm
-	// Function is "creditTo(address from, uint256 value)"
-	// selector is first 4 bytes of keccak256 of "creditTo(address,uint256)"
-	// Source:
-	// pip3 install pyethereum
-	// python3 -c 'from ethereum.utils import sha3; print(sha3("creditTo(address,uint256)")[0:4].hex())'
-	functionSelector := hexutil.MustDecode("0x9951b90c")
-	transactionData := common.GetEncodedAbi(functionSelector, [][]byte{common.AddressToAbi(address), common.AmountToAbi(amount)})
+	// Function is "creditGasFees(address,address,address,address,uint256,uint256,uint256,uint256)"
+	functionSelector := hexutil.MustDecode("0x6a30b253")
+	transactionData := common.GetEncodedAbi(functionSelector, [][]byte{common.AddressToAbi(from), common.AddressToAbi(feeRecipient), common.AddressToAbi(*gatewayFeeRecipient), common.AddressToAbi(*communityFund), common.AmountToAbi(refund), common.AmountToAbi(tipTxFee), common.AmountToAbi(gatewayFee), common.AmountToAbi(baseTxFee)})
+
 	rootCaller := vm.AccountRef(common.HexToAddress("0x0"))
 	// The caller was already charged for the cost of this operation via IntrinsicGas.
-	_, leftoverGas, err := evm.Call(rootCaller, *feeCurrency, transactionData, params.MaxGasForCreditToTransactions, big.NewInt(0))
-	gasUsed := params.MaxGasForCreditToTransactions - leftoverGas
-	log.Debug("creditTo called", "feeCurrency", *feeCurrency, "gasUsed", gasUsed)
+	_, leftoverGas, err := evm.Call(rootCaller, *feeCurrency, transactionData, params.MaxGasForCreditGasFeesTransactions, big.NewInt(0))
+	gasUsed := params.MaxGasForCreditGasFeesTransactions - leftoverGas
+	log.Trace("creditGas called", "feeCurrency", *feeCurrency, "gasUsed", gasUsed)
 	return err
 }
 
@@ -282,17 +285,7 @@ func (st *StateTransition) debitFee(from common.Address, amount *big.Int, feeCur
 		st.state.SubBalance(from, amount)
 		return nil
 	} else {
-		return st.debitFrom(from, amount, feeCurrency)
-	}
-}
-
-func (st *StateTransition) creditFee(to common.Address, amount *big.Int, feeCurrency *common.Address) (err error) {
-	// native currency
-	if feeCurrency == nil {
-		st.state.AddBalance(to, amount)
-		return nil
-	} else {
-		return st.creditTo(to, amount, feeCurrency)
+		return st.debitGas(from, amount, feeCurrency)
 	}
 }
 
@@ -395,45 +388,47 @@ func (st *StateTransition) distributeTxFees() error {
 	refund := new(big.Int).Mul(new(big.Int).SetUint64(st.gas), st.gasPrice)
 	gasUsed := new(big.Int).SetUint64(st.gasUsed())
 	totalTxFee := new(big.Int).Mul(gasUsed, st.gasPrice)
+	from := st.msg.From()
 
 	// Divide the transaction into a base (the minimum transaction fee) and tip (any extra).
 	baseTxFee := new(big.Int).Mul(gasUsed, st.gasPriceMinimum)
 	tipTxFee := new(big.Int).Sub(totalTxFee, baseTxFee)
+	feeCurrency := st.msg.FeeCurrency()
 
-	// Pay gateway fee to the specified recipient.
-	if st.msg.GatewayFeeRecipient() != nil {
-		log.Trace("Crediting gateway fee", "recipient", *st.msg.GatewayFeeRecipient(), "amount", st.msg.GatewayFee(), "feeCurrency", st.msg.FeeCurrency())
-		if err := st.creditFee(*st.msg.GatewayFeeRecipient(), st.msg.GatewayFee(), st.msg.FeeCurrency()); err != nil {
-			log.Error("Failed to credit gateway fee", "err", err)
-			return err
-		}
+	gatewayFeeRecipient := st.msg.GatewayFeeRecipient()
+	if gatewayFeeRecipient == nil {
+		gatewayFeeRecipient = &common.ZeroAddress
 	}
 
-	log.Trace("Crediting gas fee tip", "recipient", st.evm.Coinbase, "amount", tipTxFee, "feeCurrency", st.msg.FeeCurrency())
-	if err := st.creditFee(st.evm.Coinbase, tipTxFee, st.msg.FeeCurrency()); err != nil {
+	governanceAddress, err := vm.GetRegisteredAddressWithEvm(params.GovernanceRegistryId, st.evm)
+	if err != nil && err != commerrs.ErrSmartContractNotDeployed && err != commerrs.ErrRegistryContractNotDeployed {
 		return err
+	} else if err != nil {
+		log.Trace("Cannot credit gas fee to community fund: refunding fee to sender", "error", err, "fee", baseTxFee)
+		governanceAddress = &common.ZeroAddress
+		refund.Add(refund, baseTxFee)
+		baseTxFee = new(big.Int)
 	}
 
-	// Send the base of the transaction fee to the community fund.
-	governanceAddress, err := vm.GetRegisteredAddressWithEvm(params.GovernanceRegistryId, st.evm)
-	if err != nil {
-		if err != commerrs.ErrSmartContractNotDeployed && err != commerrs.ErrRegistryContractNotDeployed {
-			return err
+	log.Trace("distributeTxFees", "from", from, "refund", refund, "feeCurrency", st.msg.FeeCurrency(),
+		"gatewayFeeRecipient", *gatewayFeeRecipient, "gatewayFee", st.msg.GatewayFee(),
+		"coinbaseFeeRecipient", st.evm.Coinbase, "coinbaseFee", tipTxFee,
+		"comunityFundRecipient", *governanceAddress, "communityFundFee", baseTxFee)
+	if feeCurrency == nil {
+		if gatewayFeeRecipient != &common.ZeroAddress {
+			st.state.AddBalance(*gatewayFeeRecipient, st.msg.GatewayFee())
 		}
-		log.Trace("Cannot credit gas fee to community fund: refunding fee to sender", "error", err, "fee", baseTxFee)
-		refund.Add(refund, baseTxFee)
+		if governanceAddress != &common.ZeroAddress {
+			st.state.AddBalance(*governanceAddress, baseTxFee)
+		}
+		st.state.AddBalance(st.evm.Coinbase, tipTxFee)
+		st.state.AddBalance(from, refund)
 	} else {
-		log.Trace("Crediting gas fee tip", "recipient", *governanceAddress, "amount", baseTxFee, "feeCurrency", st.msg.FeeCurrency())
-		if err = st.creditFee(*governanceAddress, baseTxFee, st.msg.FeeCurrency()); err != nil {
+		if err = st.creditGasFees(from, st.evm.Coinbase, gatewayFeeRecipient, governanceAddress, refund, tipTxFee, st.msg.GatewayFee(), baseTxFee, feeCurrency); err != nil {
+			log.Error("Error crediting", "from", from, "coinbase", st.evm.Coinbase, "gateway", gatewayFeeRecipient, "fund", governanceAddress)
 			return err
 		}
-	}
 
-	log.Trace("Crediting refund", "recipient", st.msg.From(), "amount", refund, "feeCurrency", st.msg.FeeCurrency())
-	err = st.creditFee(st.msg.From(), refund, st.msg.FeeCurrency())
-	if err != nil {
-		log.Error("Failed to refund gas", "err", err)
-		return err
 	}
 	return nil
 }

go.sum

@@ -38,6 +38,8 @@ github.com/aristanetworks/goarista v0.0.0-20170210015632-ea17b1a17847/go.mod h1:
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/btcsuite/btcd v0.0.0-20171128150713-2e60448ffcc6 h1:Eey/GGQ/E5Xp1P2Lyx1qj007hLZfbi0+CoVeJruGCtI=
 github.com/btcsuite/btcd v0.0.0-20171128150713-2e60448ffcc6/go.mod h1:Dmm/EzmjnCiweXmzRIAiUWCInVmPgjkzgv5k4tVyXiQ=
+github.com/celo-org/gosigar v0.10.5-celo1 h1:LbhCsvNot586MAVvGFVF4cekBAo1pAYfUxktl8VuPas=
+github.com/celo-org/gosigar v0.10.5-celo1/go.mod h1:/kPo9MOBSowZbtkqUg0tJ048OJJVjG8dpaHKwAgBLz4=
 github.com/cespare/cp v0.1.0 h1:SE+dxFebS7Iik5LK0tsi1k9ZCxEaFX4AjQmoyA+1dJk=
 github.com/cespare/cp v0.1.0/go.mod h1:SOGHArjBr4JWaSDEVpWpo/hNg6RoKrls6Oh40hiwW+s=
 github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko=

params/protocol_params.go

@@ -201,8 +201,8 @@ const (
 	MaxGasForCalculateTargetEpochPaymentAndRewards uint64 = 2000000
 	MaxGasForCommitments                           uint64 = 2000000
 	MaxGasForComputeCommitment                     uint64 = 2000000
-	MaxGasForCreditToTransactions                  uint64 = 100000
-	MaxGasForDebitFromTransactions                 uint64 = 100000
+	MaxGasForDebitGasFeesTransactions              uint64 = 1000000
+	MaxGasForCreditGasFeesTransactions             uint64 = 1000000
 	MaxGasForDistributeEpochPayment                uint64 = 1 * 1000000
 	MaxGasForDistributeEpochRewards                uint64 = 1 * 1000000
 	MaxGasForElectValidators                       uint64 = 50 * 1000000