Enable smart contract calling from LES (ethereum#207)

* initial checkin for reading validator set from a smart contract. Still a WIP. * Implement currency conversion in the PriceComparator * changes for retrieving validator set from sc and saving validator set diff in the headers * fixed the golang test to work * fixed a few bugs * fixed a bug * fixed some tests * fixed lint errors * fixed error in snapshot application and modified snapshot test to test validator set diffs in the header * added another testcase for the snapshot * added test for validator set diff function * fixed lint issues * Calculate gasprice base on multiple-currency txs Calculate the gasprice for the gasprice Oracle in gold based on txs which paid gasprice in multiple currencies. * changed istanbul acceptors to validate block and changes based on PR. * disabled a few test cases (for now) * Implement currency conversion in the PriceComparator * Calculate gasprice base on multiple-currency txs Calculate the gasprice for the gasprice Oracle in gold based on txs which paid gasprice in multiple currencies. * Remove gpo from light client * Add internal EVM handler to Istanbul Backend * Generalize EVM signature and add to the Ethereum object * Add InternalEVMHandler to Light Ethereum Struct * Call GasPriceOracle SC from Geth * Add gaspricesuggestion update function to Finalize * Add SC call to LES api_backend * Add an address registry check for newly connected light clients * Clean up logging * Remove unrelated changes from rebase * Clean up some code placement and TODOs for PR * Clean up some code placement and TODOs for PR * Reorder function for readability * Reorder function for readability * Simplify iEvmH constructions * Edits based on gofmt * Edit test Istanbul signatures * Edit test Istanbul signatures * Fix linting error * Refactor comments and implement other small PR change requests * Simplify EVM abstraction with better use of blockchain objects * Implement formatting fixes from gofmt * Move business logic out of api_backend * Edits based on gofmt * Add copyright info to new file * Fix lightchain signatures in tests * Fix nil currency error * Add logging for conversion failure * Respond to PR comments * Respond to PR comments * Implement changes in response to PR * Remove iEvmH from Consensus Engine Construction Remove iEvmH from Consensus Engine Construction in order to simplify signature and remove duplicate iEvmH setting. * Edits based on gofmt * Fix test signatures * Remove whitespace * Update ChainContext comment * Edits based on gofmt * Refactor Price Comparator and remove vmConfig from lightchain * Add address refresh on les construction and remove func * Respond to PR comments * Handle concurrency in CurrencyOperator * Add temporary test logging * Remove test logging statements * Add comment for registered address refresh
tony-ricciardi · Jun 4, 2019 · 348e076 · 348e076
1 parent acfe0e6
commit 348e076
Show file tree

Hide file tree

Showing 19 changed files with 315 additions and 160 deletions.
diff --git a/consensus/istanbul/backend/engine.go b/consensus/istanbul/backend/engine.go
@@ -428,6 +428,10 @@ func (sb *Backend) IsLastBlockOfEpoch(header *types.Header) bool {
 // consensus rules that happen at finalization (e.g. block rewards).
 func (sb *Backend) Finalize(chain consensus.ChainReader, header *types.Header, state *state.StateDB, txs []*types.Transaction,
 	uncles []*types.Header, receipts []*types.Receipt) (*types.Block, error) {
+
+	// Calculate a new gas price suggestion and push it to the GasPriceOracle SmartContract
+	sb.updateGasPriceSuggestion(state)
+
 	// No block rewards in Istanbul, so the state remains as is and uncles are dropped
 	header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
 	header.UncleHash = nilUncleHash
@@ -436,6 +440,11 @@ func (sb *Backend) Finalize(chain consensus.ChainReader, header *types.Header, s
 	return types.NewBlock(header, txs, nil, receipts), nil
 }
 
+// TODO (jarmg 5/23/18): Implement this
+func (sb *Backend) updateGasPriceSuggestion(state *state.StateDB) *state.StateDB {
+	return (state)
+}
+
 // Seal generates a new block for the given input block with the local miner's
 // seal place on top.
 func (sb *Backend) Seal(chain consensus.ChainReader, block *types.Block, results chan<- *types.Block, stop <-chan struct{}) error {

diff --git a/core/currency.go b/core/currency.go
@@ -109,33 +109,66 @@ type exchangeRate struct {
 	Denominator *big.Int
 }
 
-type PriceComparator struct {
-	gcWl          *GasCurrencyWhitelist            // Object to retrieve the set of currencies that will have their exchange rate monitored
-	exchangeRates map[common.Address]*exchangeRate // indexedCurrency:CeloGold exchange rate
-	regAdd        *RegisteredAddresses
-	iEvmH         *InternalEVMHandler
+type CurrencyOperator struct {
+	gcWl               *GasCurrencyWhitelist            // Object to retrieve the set of currencies that will have their exchange rate monitored
+	exchangeRates      map[common.Address]*exchangeRate // indexedCurrency:CeloGold exchange rate
+	regAdd             *RegisteredAddresses
+	iEvmH              *InternalEVMHandler
+	currencyOperatorMu sync.RWMutex
 }
 
-// Returns the price of gold in the provided currency.
-func (pc *PriceComparator) getExchangeRate(currency *common.Address) (*big.Int, *big.Int, error) {
+func (co *CurrencyOperator) getExchangeRate(currency *common.Address) (*exchangeRate, error) {
 	if currency == nil {
-		return cgExchangeRateNum, cgExchangeRateDen, nil
+		return &exchangeRate{cgExchangeRateNum, cgExchangeRateDen}, nil
 	} else {
-		if exchangeRate, ok := pc.exchangeRates[*currency]; !ok {
-			return nil, nil, errExchangeRateCacheMiss
+		co.currencyOperatorMu.RLock()
+		defer co.currencyOperatorMu.RUnlock()
+		if exchangeRate, ok := co.exchangeRates[*currency]; !ok {
+			return nil, errExchangeRateCacheMiss
 		} else {
-			return exchangeRate.Numerator, exchangeRate.Denominator, nil
+			return exchangeRate, nil
 		}
 	}
 }
 
-func (pc *PriceComparator) Cmp(val1 *big.Int, currency1 *common.Address, val2 *big.Int, currency2 *common.Address) int {
+func (co *CurrencyOperator) ConvertToGold(val *big.Int, currencyFrom *common.Address) (*big.Int, error) {
+	celoGoldAddress := co.regAdd.GetRegisteredAddress(params.GoldTokenRegistryId)
+	if currencyFrom == nil || currencyFrom == celoGoldAddress {
+		return val, nil
+	}
+	return co.Convert(val, currencyFrom, celoGoldAddress)
+}
+
+// NOTE (jarmg 4/24/18): values are rounded down which can cause
+// an estimate to be off by 1 (at most)
+func (co *CurrencyOperator) Convert(val *big.Int, currencyFrom *common.Address, currencyTo *common.Address) (*big.Int, error) {
+	exchangeRateFrom, err1 := co.getExchangeRate(currencyFrom)
+	exchangeRateTo, err2 := co.getExchangeRate(currencyTo)
+
+	if err1 != nil || err2 != nil {
+		log.Error("CurrencyOperator.Convert - Error in retreiving currency exchange rates")
+		if err1 != nil {
+			return nil, err1
+		}
+		if err2 != nil {
+			return nil, err2
+		}
+	}
+
+	// Given value of val and rates n1/d1 and n2/d2 the function below does
+	// (val * n1 * d2) / (d1 * n2)
+	numerator := new(big.Int).Mul(val, new(big.Int).Mul(exchangeRateFrom.Numerator, exchangeRateTo.Denominator))
+	denominator := new(big.Int).Mul(exchangeRateFrom.Denominator, exchangeRateTo.Numerator)
+	return new(big.Int).Div(numerator, denominator), nil
+}
+
+func (co *CurrencyOperator) Cmp(val1 *big.Int, currency1 *common.Address, val2 *big.Int, currency2 *common.Address) int {
 	if currency1 == currency2 {
 		return val1.Cmp(val2)
 	}
 
-	exchangeRate1Num, exchangeRate1Den, err1 := pc.getExchangeRate(currency1)
-	exchangeRate2Num, exchangeRate2Den, err2 := pc.getExchangeRate(currency2)
+	exchangeRate1, err1 := co.getExchangeRate(currency1)
+	exchangeRate2, err2 := co.getExchangeRate(currency2)
 
 	if err1 != nil || err2 != nil {
 		currency1Output := "nil"
@@ -151,85 +184,90 @@ func (pc *PriceComparator) Cmp(val1 *big.Int, currency1 *common.Address, val2 *b
 	}
 
 	// Below code block is basically evaluating this comparison:
-	// val1 / exchangeRate1Num/exchangeRate1Den < val2 / exchangeRate2Num/exchangeRate2Den
+	// val1 * exchangeRate1.Numerator/exchangeRate1.Denominator < val2 * exchangeRate2.Numerator/exchangeRate2.Denominator
 	// It will transform that comparison to this, to remove having to deal with fractional values.
-	// val1 * exchangeRate2Num * exchangeRate1Den < val2 * exchangeRate1Num * exchangeRate2Den
-	leftSide := new(big.Int).Mul(val1, new(big.Int).Mul(exchangeRate2Num, exchangeRate1Den))
-	rightSide := new(big.Int).Mul(val2, new(big.Int).Mul(exchangeRate1Num, exchangeRate2Den))
+	// val1 * exchangeRate1.Numerator * exchangeRate2.Denominator < val2 * exchangeRate2.Numerator * exchangeRate1.Denominator
+	leftSide := new(big.Int).Mul(val1, new(big.Int).Mul(exchangeRate1.Numerator, exchangeRate2.Denominator))
+	rightSide := new(big.Int).Mul(val2, new(big.Int).Mul(exchangeRate2.Numerator, exchangeRate1.Denominator))
 	return leftSide.Cmp(rightSide)
 }
 
 // This function will retrieve the exchange rates from the SortedOracles contract and cache them.
 // SortedOracles must have a function with the following signature:
 // "function medianRate(address)"
-func (pc *PriceComparator) retrieveExchangeRates() {
-	gasCurrencyAddresses := pc.gcWl.retrieveWhitelist()
-	log.Trace("PriceComparator.retrieveExchangeRates called", "gasCurrencyAddresses", gasCurrencyAddresses)
+func (co *CurrencyOperator) retrieveExchangeRates() {
+	gasCurrencyAddresses := co.gcWl.retrieveWhitelist()
+	log.Trace("CurrencyOperator.retrieveExchangeRates called", "gasCurrencyAddresses", gasCurrencyAddresses)
 
-	sortedOraclesAddress := pc.regAdd.GetRegisteredAddress(params.SortedOraclesRegistryId)
+	sortedOraclesAddress := co.regAdd.GetRegisteredAddress(params.SortedOraclesRegistryId)
 
 	if sortedOraclesAddress == nil {
 		log.Error("Can't get the sortedOracles smart contract address from the registry")
 		return
 	}
 
-	celoGoldAddress := pc.regAdd.GetRegisteredAddress(params.GoldTokenRegistryId)
+	celoGoldAddress := co.regAdd.GetRegisteredAddress(params.GoldTokenRegistryId)
 
 	if celoGoldAddress == nil {
 		log.Error("Can't get the celo gold smart contract address from the registry")
 		return
 	}
 
+	co.currencyOperatorMu.Lock()
+
 	for _, gasCurrencyAddress := range gasCurrencyAddresses {
 		if gasCurrencyAddress == *celoGoldAddress {
 			continue
 		}
 
 		var returnArray [2]*big.Int
 
-		log.Trace("PriceComparator.retrieveExchangeRates - Calling medianRate", "sortedOraclesAddress", sortedOraclesAddress.Hex(),
+		log.Trace("CurrencyOperator.retrieveExchangeRates - Calling medianRate", "sortedOraclesAddress", sortedOraclesAddress.Hex(),
 			"gas currency", gasCurrencyAddress.Hex())
 
-		if leftoverGas, err := pc.iEvmH.MakeCall(*sortedOraclesAddress, medianRateFuncABI, "medianRate", []interface{}{gasCurrencyAddress}, &returnArray, 20000, nil, nil); err != nil {
-			log.Error("PriceComparator.retrieveExchangeRates - SortedOracles.medianRate invocation error", "leftoverGas", leftoverGas, "err", err)
+		if leftoverGas, err := co.iEvmH.MakeCall(*sortedOraclesAddress, medianRateFuncABI, "medianRate", []interface{}{gasCurrencyAddress}, &returnArray, 20000, nil, nil); err != nil {
+			log.Error("CurrencyOperator.retrieveExchangeRates - SortedOracles.medianRate invocation error", "leftoverGas", leftoverGas, "err", err)
 			continue
 		} else {
-			log.Trace("PriceComparator.retrieveExchangeRates - SortedOracles.medianRate invocation success", "returnArray", returnArray, "leftoverGas", leftoverGas)
+			log.Trace("CurrencyOperator.retrieveExchangeRates - SortedOracles.medianRate invocation success", "returnArray", returnArray, "leftoverGas", leftoverGas)
 
-			if _, ok := pc.exchangeRates[gasCurrencyAddress]; !ok {
-				pc.exchangeRates[gasCurrencyAddress] = &exchangeRate{}
+			if _, ok := co.exchangeRates[gasCurrencyAddress]; !ok {
+				co.exchangeRates[gasCurrencyAddress] = &exchangeRate{}
 			}
 
-			pc.exchangeRates[gasCurrencyAddress].Numerator = returnArray[0]
-			pc.exchangeRates[gasCurrencyAddress].Denominator = returnArray[1]
+			co.exchangeRates[gasCurrencyAddress].Numerator = returnArray[0]
+			co.exchangeRates[gasCurrencyAddress].Denominator = returnArray[1]
 		}
 	}
+
+	co.currencyOperatorMu.Unlock()
 }
 
-func (pc *PriceComparator) mainLoop() {
-	pc.retrieveExchangeRates()
+// TODO (jarmg 5/30/18): Change this to cache based on block number
+func (co *CurrencyOperator) mainLoop() {
+	co.retrieveExchangeRates()
 	ticker := time.NewTicker(10 * time.Second)
 
 	for range ticker.C {
-		pc.retrieveExchangeRates()
+		co.retrieveExchangeRates()
 	}
 }
 
-func NewPriceComparator(gcWl *GasCurrencyWhitelist, regAdd *RegisteredAddresses, iEvmH *InternalEVMHandler) *PriceComparator {
+func NewCurrencyOperator(gcWl *GasCurrencyWhitelist, regAdd *RegisteredAddresses, iEvmH *InternalEVMHandler) *CurrencyOperator {
 	exchangeRates := make(map[common.Address]*exchangeRate)
 
-	pc := &PriceComparator{
+	co := &CurrencyOperator{
 		gcWl:          gcWl,
 		exchangeRates: exchangeRates,
 		regAdd:        regAdd,
 		iEvmH:         iEvmH,
 	}
 
-	if pc.gcWl != nil {
-		go pc.mainLoop()
+	if co.gcWl != nil {
+		go co.mainLoop()
 	}
 
-	return pc
+	return co
 }
 
 // This function will retrieve the balance of an ERC20 token.

diff --git a/core/evm.go b/core/evm.go
@@ -29,14 +29,24 @@ import (
 	"github.com/ethereum/go-ethereum/params"
 )
 
-// ChainContext supports retrieving headers and consensus parameters from the
-// current blockchain to be used during transaction processing.
+// ChainContext supports retrieving chain data and consensus parameters
+// from the block chain to be used during transaction processing.
 type ChainContext interface {
-	// Engine retrieves the chain's consensus engine.
+	// Engine retrieves the blockchain's consensus engine.
 	Engine() consensus.Engine
 
 	// GetHeader returns the hash corresponding to their hash.
 	GetHeader(common.Hash, uint64) *types.Header
+
+	// GetVMConfig returns the node's vm configuration
+	GetVMConfig() *vm.Config
+
+	CurrentHeader() *types.Header
+
+	State() (*state.StateDB, error)
+
+	// Config returns the blockchain's chain configuration
+	Config() *params.ChainConfig
 }
 
 // NewEVMContext creates a new context for use in the EVM.
@@ -138,24 +148,24 @@ func Transfer(db vm.StateDB, sender, recipient common.Address, amount *big.Int)
 
 // An EVM handler to make calls to smart contracts from within geth
 type InternalEVMHandler struct {
-	blockchain  *BlockChain         // Used to construct the EVM object needed to make the call the medianator contract
-	chainConfig *params.ChainConfig // The config object of the eth object
-	regAdd      *RegisteredAddresses
+	chain  ChainContext
+	regAdd *RegisteredAddresses
 }
 
 func (iEvmH *InternalEVMHandler) MakeCall(scAddress common.Address, abi abi.ABI, funcName string, args []interface{}, returnObj interface{}, gas uint64, header *types.Header, state *state.StateDB) (uint64, error) {
 	// Normally, when making an evm call, we should use the current block's state.  However,
 	// there are times (e.g. retrieving the set of validators when an epoch ends) that we need
 	// to call the evm using the currently mined block.  In that case, the header and state params
 	// will be non nil.
+	log.Trace("InternalEVMHandler.MakeCall called")
 
 	if header == nil {
-		header = iEvmH.blockchain.CurrentBlock().Header()
+		header = iEvmH.chain.CurrentHeader()
 	}
 
 	if state == nil {
 		var err error
-		state, err = iEvmH.blockchain.StateAt(header.Root)
+		state, err = iEvmH.chain.State()
 		if err != nil {
 			log.Error("Error in retrieving the state from the blockchain")
 			return 0, err
@@ -165,8 +175,8 @@ func (iEvmH *InternalEVMHandler) MakeCall(scAddress common.Address, abi abi.ABI,
 	// The EVM Context requires a msg, but the actual field values don't really matter for this case.
 	// Putting in zero values.
 	msg := types.NewMessage(common.HexToAddress("0x0"), nil, 0, common.Big0, 0, common.Big0, nil, nil, []byte{}, false)
-	context := NewEVMContext(msg, header, iEvmH.blockchain, nil, iEvmH.regAdd)
-	evm := vm.NewEVM(context, state, iEvmH.chainConfig, *iEvmH.blockchain.GetVMConfig())
+	context := NewEVMContext(msg, header, iEvmH.chain, nil, iEvmH.regAdd)
+	evm := vm.NewEVM(context, state, iEvmH.chain.Config(), *iEvmH.chain.GetVMConfig())
 
 	zeroCaller := vm.AccountRef(common.HexToAddress("0x0"))
 	return evm.ABIStaticCall(zeroCaller, scAddress, abi, funcName, args, returnObj, gas)
@@ -176,11 +186,9 @@ func (iEvmH *InternalEVMHandler) SetRegisteredAddresses(regAdd *RegisteredAddres
 	iEvmH.regAdd = regAdd
 }
 
-func NewInternalEVMHandler(chainConfig *params.ChainConfig, blockchain *BlockChain) *InternalEVMHandler {
+func NewInternalEVMHandler(chain ChainContext) *InternalEVMHandler {
 	iEvmH := InternalEVMHandler{
-		blockchain:  blockchain,
-		chainConfig: chainConfig,
+		chain: chain,
 	}
-
 	return &iEvmH
 }
diff --git a/core/registeredAddresses.go b/core/registeredAddresses.go
@@ -50,7 +50,7 @@ const (
 
 var (
 	registrySmartContractAddress = common.HexToAddress("0x000000000000000000000000000000000000ce10")
-	registeredContractIds        = []string{params.GoldTokenRegistryId, params.AddressBasedEncryptionRegistryId, params.ReserveRegistryId, params.SortedOraclesRegistryId, params.GasCurrencyWhitelistRegistryId, params.ValidatorsRegistryId}
+	registeredContractIds        = []string{params.GoldTokenRegistryId, params.AddressBasedEncryptionRegistryId, params.ReserveRegistryId, params.SortedOraclesRegistryId, params.GasCurrencyWhitelistRegistryId, params.ValidatorsRegistryId, params.GasPriceOracleRegistryId}
 	getAddressForFuncABI, _      = abi.JSON(strings.NewReader(getAddressForABI))
 	zeroAddress                  = common.Address{}
 )
@@ -93,10 +93,15 @@ func (ra *RegisteredAddresses) RefreshAddresses() {
 }
 
 func (ra *RegisteredAddresses) GetRegisteredAddress(registryId string) *common.Address {
+	if len(ra.registeredAddresses) == 0 { // This refresh is for a light client that failed to refresh (did not have a network connection) during node construction
+		ra.RefreshAddresses()
+	}
+
 	ra.registeredAddressesMu.RLock()
 	defer ra.registeredAddressesMu.RUnlock()
 
 	if address, ok := ra.registeredAddresses[registryId]; !ok {
+		log.Error("RegisteredAddresses.GetRegisteredAddress - Error in address retrieval for ", "registry", registryId)
 		return nil
 	} else {
 		return &address

diff --git a/core/tx_list.go b/core/tx_list.go
@@ -411,16 +411,16 @@ type txPricedList struct {
 	nonNilCurrencyHeaps map[common.Address]*priceHeap // Heap of prices of all the stored non-nil currency transactions
 	nilCurrencyHeap     *priceHeap                    // Heap of prices of all the stored nil currency transactions
 	stales              int                           // Number of stale price points to (re-heap trigger)
-	pc                  *PriceComparator              // Comparator object used to compare prices that are using different currencies
+	co                  *CurrencyOperator             // Comparator object used to compare prices that are using different currencies
 }
 
 // newTxPricedList creates a new price-sorted transaction heap.
-func newTxPricedList(all *txLookup, pc *PriceComparator) *txPricedList {
+func newTxPricedList(all *txLookup, co *CurrencyOperator) *txPricedList {
 	return &txPricedList{
 		all:                 all,
 		nonNilCurrencyHeaps: make(map[common.Address]*priceHeap),
 		nilCurrencyHeap:     new(priceHeap),
-		pc:                  pc,
+		co:                  co,
 	}
 }
 
@@ -489,7 +489,7 @@ func (l *txPricedList) Cap(cgThreshold *big.Int, local *accountSet) types.Transa
 			continue
 		}
 
-		if l.pc.Cmp(tx.GasPrice(), tx.GasCurrency(), cgThreshold, nil) >= 0 {
+		if l.co.Cmp(tx.GasPrice(), tx.GasCurrency(), cgThreshold, nil) >= 0 {
 			save = append(save, tx)
 			break
 		}
@@ -531,7 +531,7 @@ func (l *txPricedList) Underpriced(tx *types.Transaction, local *accountSet) boo
 	}
 
 	cheapest := l.getMinPricedTx()
-	return l.pc.Cmp(cheapest.GasPrice(), cheapest.GasCurrency(), tx.GasPrice(), tx.GasCurrency()) >= 0
+	return l.co.Cmp(cheapest.GasPrice(), cheapest.GasCurrency(), tx.GasPrice(), tx.GasCurrency()) >= 0
 }
 
 // Discard finds a number of most underpriced transactions, removes them from the
@@ -574,7 +574,7 @@ func (l *txPricedList) getHeapWithMinHead() (*priceHeap, *types.Transaction) {
 				cheapestTxn = []*types.Transaction(*cheapestHeap)[0]
 			} else {
 				txn := []*types.Transaction(*priceHeap)[0]
-				if l.pc.Cmp(cheapestTxn.GasPrice(), cheapestTxn.GasCurrency(), txn.GasPrice(), txn.GasCurrency()) < 0 {
+				if l.co.Cmp(cheapestTxn.GasPrice(), cheapestTxn.GasCurrency(), txn.GasPrice(), txn.GasCurrency()) < 0 {
 					cheapestHeap = priceHeap
 				}
 			}