From 038169bc8e23b48d498d56242ed4d1b8908e15cd Mon Sep 17 00:00:00 2001 From: "Victor \"Nate\" Graf" Date: Wed, 13 Nov 2019 15:18:23 -0800 Subject: [PATCH] Split gas fee into base and tip with base going to infrastructure fund (#563) * begin to split tx fees into base and tip * consolidate two lines into one * send whole base tx fee to infra fund and eliminate proposer fraction reference * fix tests to work independent of golang version * ensure nil error is returned on success * fix typo * point CI to victor/gpm * remove reference to proposer fraction * adjust comment on fallback gas price minimum * distribute the entire gas fee to the proposer when the governance address cannot be fetched * Revert "distribute the entire gas fee to the proposer when the governance address cannot be fetched" This reverts commit 543ed2978b25b4dd3c52a9c57990717275807fdd. * refund tx fee to sender if infrastructure fund addr cannot be resolved --- .circleci/config.yml | 2 +- .../gasprice_minimum/gasprice_minimum.go | 52 +-------- core/state_transition.go | 101 ++++++++++-------- les/api_backend.go | 4 - p2p/discv5/node_test.go | 4 +- p2p/enode/urlv4_test.go | 4 +- params/protocol_params.go | 1 - 7 files changed, 65 insertions(+), 103 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bd28c8b84dac..4b084cbbc91a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -69,7 +69,7 @@ jobs: command: | set -euo pipefail export CELO_MONOREPO_DIR="$PWD" - git clone --depth 1 https://github.com/celo-org/celo-monorepo.git ${CELO_MONOREPO_DIR} -b master + git clone --depth 1 https://github.com/celo-org/celo-monorepo.git ${CELO_MONOREPO_DIR} -b victor/gpm yarn install || yarn install # separate build to avoid ENOMEM in CI :( yarn build --scope @celo/utils diff --git a/contract_comm/gasprice_minimum/gasprice_minimum.go b/contract_comm/gasprice_minimum/gasprice_minimum.go index 6716a5c8d829..917a2a68d034 100644 --- a/contract_comm/gasprice_minimum/gasprice_minimum.go +++ b/contract_comm/gasprice_minimum/gasprice_minimum.go @@ -33,24 +33,6 @@ import ( // TODO (jarmg 5/22/19): Store ABIs in a central location const ( gasPriceMinimumABIString = `[ - { - "constant": true, - "inputs": [], - "name": "proposerFraction_", - "outputs": [ - { - "name": "", - "type": "uint256" - }, - { - "name": "", - "type": "uint256" - } - ], - "payable": false, - "stateMutability": "view", - "type": "function" - }, { "constant": true, "inputs": [ @@ -98,17 +80,11 @@ const ( ) var ( - gasPriceMinimumABI, _ = abi.JSON(strings.NewReader(gasPriceMinimumABIString)) - FallbackProposerFraction *ProposerFraction = &ProposerFraction{big.NewInt(0), big.NewInt(1)} - FallbackGasPriceMinimum *big.Int = big.NewInt(0) // gasprice min to return if contracts are not found - suggestionMultiplier *big.Int = big.NewInt(5) // The multiplier that we apply to the minimum when suggesting gas price + gasPriceMinimumABI, _ = abi.JSON(strings.NewReader(gasPriceMinimumABIString)) + FallbackGasPriceMinimum *big.Int = big.NewInt(0) // gas price minimum to return if unable to fetch from contract + suggestionMultiplier *big.Int = big.NewInt(5) // The multiplier that we apply to the minimum when suggesting gas price ) -type ProposerFraction struct { - Numerator *big.Int - Denominator *big.Int -} - func GetGasPriceSuggestion(currency *common.Address, header *types.Header, state vm.StateDB) (*big.Int, error) { gasPriceMinimum, err := GetGasPriceMinimum(currency, header, state) return new(big.Int).Mul(gasPriceMinimum, suggestionMultiplier), err @@ -176,25 +152,3 @@ func UpdateGasPriceMinimum(header *types.Header, state vm.StateDB) (*big.Int, er } return updatedGasPriceMinimum, err } - -// Returns the fraction of the gasprice min that should be allocated to the proposer -func GetProposerFraction(header *types.Header, state vm.StateDB) (*ProposerFraction, error) { - infraFraction := [2]*big.Int{big.NewInt(0), big.NewInt(1)} // Give everything to the miner as Fallback - - _, err := contract_comm.MakeStaticCall( - params.GasPriceMinimumRegistryId, - gasPriceMinimumABI, - "proposerFraction_", - []interface{}{}, - &infraFraction, - params.MaxGasForProposerFraction, - header, - state, - ) - - if err != nil { - return FallbackProposerFraction, err - } - - return &ProposerFraction{infraFraction[0], infraFraction[1]}, err -} diff --git a/core/state_transition.go b/core/state_transition.go index b4c4d71851a4..f80ab1a3ce38 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -25,6 +25,7 @@ import ( "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/contract_comm/blockchain_parameters" "github.com/ethereum/go-ethereum/contract_comm/currency" + commerrs "github.com/ethereum/go-ethereum/contract_comm/errors" gpm "github.com/ethereum/go-ethereum/contract_comm/gasprice_minimum" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" @@ -57,17 +58,16 @@ The state transitioning model does all the necessary work to work out a valid ne 6) Derive new state root */ type StateTransition struct { - gp *GasPool - msg Message - gas uint64 - gasPrice *big.Int - initialGas uint64 - value *big.Int - data []byte - state vm.StateDB - evm *vm.EVM - gasPriceMinimum *big.Int - proposerFraction *gpm.ProposerFraction + gp *GasPool + msg Message + gas uint64 + gasPrice *big.Int + initialGas uint64 + value *big.Int + data []byte + state vm.StateDB + evm *vm.EVM + gasPriceMinimum *big.Int } // Message represents a message sent to a contract. @@ -81,6 +81,7 @@ type Message interface { // nil correspond to Celo Gold (native currency). // All other values should correspond to ERC20 contract addresses extended to be compatible with gas payments. GasCurrency() *common.Address + // TODO: GasFeeRecipient is currently inactive and will be replaced by GatewayFeeRecipient. GasFeeRecipient() *common.Address Value() *big.Int @@ -142,18 +143,16 @@ func IntrinsicGas(data []byte, contractCreation, homestead bool, header *types.H // NewStateTransition initialises and returns a new state transition object. func NewStateTransition(evm *vm.EVM, msg Message, gp *GasPool) *StateTransition { gasPriceMinimum, _ := gpm.GetGasPriceMinimum(msg.GasCurrency(), evm.GetHeader(), evm.GetStateDB()) - proposerFraction, _ := gpm.GetProposerFraction(evm.GetHeader(), evm.GetStateDB()) return &StateTransition{ - gp: gp, - evm: evm, - msg: msg, - gasPrice: msg.GasPrice(), - value: msg.Value(), - data: msg.Data(), - state: evm.StateDB, - gasPriceMinimum: gasPriceMinimum, - proposerFraction: proposerFraction, + gp: gp, + evm: evm, + msg: msg, + gasPrice: msg.GasPrice(), + value: msg.Value(), + data: msg.Data(), + state: evm.StateDB, + gasPriceMinimum: gasPriceMinimum, } } @@ -296,7 +295,7 @@ func (st *StateTransition) preCheck() error { // Make sure this transaction's gas price is valid. if st.gasPrice.Cmp(st.gasPriceMinimum) < 0 { - log.Error("Tx gas price does not exceed minimum", "minimum", st.gasPriceMinimum, "price", st.gasPrice) + log.Error("Tx gas price is less than minimum", "minimum", st.gasPriceMinimum, "price", st.gasPrice) return ErrGasPriceDoesNotExceedMinimum } @@ -367,37 +366,55 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo } } - // Distribute transaction fees - err = st.refundGas() + st.refundGas() + + err = st.distributeTxFees() if err != nil { - log.Error("Failed to refund gas", "err", err) return nil, 0, false, err } - // Pay tx fee to tx fee recipient and the proposer (coinbase) - txFeeRecipient := msg.GasFeeRecipient() - if txFeeRecipient == nil { - sender := msg.From() - txFeeRecipient = &sender - } + return ret, st.gasUsed(), vmerr != nil, nil +} +// distributeTxFees calculates the amounts and recipients of transaction fees and credits the accounts. +func (st *StateTransition) distributeTxFees() error { + // Determine the refund and transaction fee to be distributed. + 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) - // gasUsed * gasPriceMinimun * proposerFractionNumerator / proposerFractionDenominator - coinbaseTxFee := new(big.Int).Div(new(big.Int).Mul(gasUsed, new(big.Int).Mul(st.gasPriceMinimum, st.proposerFraction.Numerator)), st.proposerFraction.Denominator) - recipientTxFee := new(big.Int).Sub(totalTxFee, coinbaseTxFee) - if err = st.creditGas(st.evm.Coinbase, coinbaseTxFee, msg.GasCurrency()); err != nil { - return nil, 0, false, err + // 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) + + if err := st.creditGas(st.evm.Coinbase, tipTxFee, st.msg.GasCurrency()); err != nil { + return err } - if err = st.creditGas(*txFeeRecipient, recipientTxFee, msg.GasCurrency()); err != nil { - return nil, 0, false, err + + // Send the base of the transaction fee to the infrastructure fund. + governanceAddress, err := vm.GetRegisteredAddressWithEvm(params.GovernanceRegistryId, st.evm) + if err != nil { + if err != commerrs.ErrSmartContractNotDeployed && err != commerrs.ErrRegistryContractNotDeployed { + return err + } + log.Trace("Cannot credit gas fee to infrastructure fund: refunding fee to sender", "error", err, "fee", baseTxFee) + refund.Add(refund, baseTxFee) + } else { + if err = st.creditGas(*governanceAddress, baseTxFee, st.msg.GasCurrency()); err != nil { + return err + } } - return ret, st.gasUsed(), vmerr != nil, err + err = st.creditGas(st.msg.From(), refund, st.msg.GasCurrency()) + if err != nil { + log.Error("Failed to refund gas", "err", err) + return err + } + return nil } -func (st *StateTransition) refundGas() error { +// refundGas adds unused gas back the state transition and gas pool. +func (st *StateTransition) refundGas() { refund := st.state.GetRefund() // Apply refund counter, capped to half of the used gas. if refund > st.gasUsed()/2 { @@ -405,13 +422,9 @@ func (st *StateTransition) refundGas() error { } st.gas += refund - // Return ETH for remaining gas, exchanged at the original rate. - remaining := new(big.Int).Mul(new(big.Int).SetUint64(st.gas), st.gasPrice) - err := st.creditGas(st.msg.From(), remaining, st.msg.GasCurrency()) // Also return remaining gas to the block gas counter so it is // available for the next transaction. st.gp.AddGas(st.gas) - return err } // gasUsed returns the amount of gas used up by the state transition. diff --git a/les/api_backend.go b/les/api_backend.go index e6d00734625d..f9b589c1d2e9 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -183,10 +183,6 @@ func (b *LesApiBackend) GetGasPriceMinimum(ctx context.Context, currencyAddress return gpm.GetGasPriceMinimum(currencyAddress, nil, nil) } -func (b *LesApiBackend) ProposerFraction(ctx context.Context) (*gpm.ProposerFraction, error) { - return gpm.GetProposerFraction(nil, nil) -} - func (b *LesApiBackend) ChainDb() ethdb.Database { return b.eth.chainDb } diff --git a/p2p/discv5/node_test.go b/p2p/discv5/node_test.go index afed1faa3f98..439c5a64f156 100644 --- a/p2p/discv5/node_test.go +++ b/p2p/discv5/node_test.go @@ -73,7 +73,7 @@ var parseNodeTests = []struct { { rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:foo", // net/url.Parse(rawurl) returns an error with rawurl and why the parse failed. - wantError: `parse enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:foo: invalid port ":foo" after host`, + wantError: `invalid port`, }, { rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:3?discport=foo", @@ -153,7 +153,7 @@ func TestParseNode(t *testing.T) { if err == nil { t.Errorf("test %q:\n got nil error, expected %#q", test.rawurl, test.wantError) continue - } else if err.Error() != test.wantError { + } else if !strings.Contains(err.Error(), test.wantError) { t.Errorf("test %q:\n got error %#q, expected %#q", test.rawurl, err.Error(), test.wantError) continue } diff --git a/p2p/enode/urlv4_test.go b/p2p/enode/urlv4_test.go index e7e04c12bf4f..a4294f6aeaeb 100644 --- a/p2p/enode/urlv4_test.go +++ b/p2p/enode/urlv4_test.go @@ -48,7 +48,7 @@ var parseNodeTests = []struct { { // net/url.Parse(rawurl) returns an error with rawurl and why the parse failed. rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:foo", - wantError: `parse enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:foo: invalid port ":foo" after host`, + wantError: `invalid port`, }, { rawurl: "enode://1dd9d65c4552b5eb43d5ad55a2ee3f56c6cbc1c64a5c8d659f51fcd51bace24351232b8d7821617d2b29b54b81cdefb9b3e9c37d7fd5f63270bcc9e1a6f6a439@127.0.0.1:3?discport=foo", @@ -136,7 +136,7 @@ func TestParseNode(t *testing.T) { if err == nil { t.Errorf("test %q:\n got nil error, expected %#q", test.rawurl, test.wantError) continue - } else if err.Error() != test.wantError { + } else if !strings.Contains(err.Error(), test.wantError) { t.Errorf("test %q:\n got error %#q, expected %#q", test.rawurl, err.Error(), test.wantError) continue } diff --git a/params/protocol_params.go b/params/protocol_params.go index 681527d9fc34..e2d0facca6b5 100644 --- a/params/protocol_params.go +++ b/params/protocol_params.go @@ -155,7 +155,6 @@ const ( MaxGasForGetWhiteList uint64 = 20000 MaxGasForIncreaseSupply uint64 = 50 * 1000 MaxGasForMedianRate uint64 = 20000 - MaxGasForProposerFraction uint64 = 200000 MaxGasForReadBlockchainParameter uint64 = 20000 MaxGasForRevealAndCommit uint64 = 2000000 MaxGasForUpdateGasPriceMinimum uint64 = 2000000