core/beacon, eth: clean up engine API returns a bit

core/beacon/errors.go

@@ -19,10 +19,25 @@ package beacon
 import "github.com/ethereum/go-ethereum/rpc"
 
 var (
-	VALID              = GenericStringResponse{"VALID"}
-	SUCCESS            = GenericStringResponse{"SUCCESS"}
-	INVALID            = ForkChoiceResponse{Status: "INVALID", PayloadID: nil}
-	SYNCING            = ForkChoiceResponse{Status: "SYNCING", PayloadID: nil}
+	// VALID is returned by the engine API in the following calls:
+	//   - newPayloadV1:       if the payload was already known or was just validated and executed
+	//   - forkchoiceUpdateV1: if the chain accepted the reorg (might ignore if it's stale)
+	VALID = "VALID"
+
+	// INVALID is returned by the engine API in the following calls:
+	//   - newPayloadV1:       if the payload failed to execute on top of the local chain
+	//   - forkchoiceUpdateV1: if the new head is unknown, pre-merge, or reorg to it fails
+	INVALID = "INVALID"
+
+	// SYNCING is returned by the engine API in the following calls:
+	//   - newPayloadV1:       if the payload was accepted on top of an active sync
+	//   - forkchoiceUpdateV1: if the new head was seen before, but not part of the chain
+	SYNCING = "SYNCING"
+
+	// ACCEPTED is returned by the engine API in the following calls:
+	//   - newPayloadV1: if the payload was accepted, but not processed (side chain)
+	ACCEPTED = "ACCEPTED"
+
 	GenericServerError = rpc.CustomError{Code: -32000, ValidationError: "Server error"}
 	UnknownPayload     = rpc.CustomError{Code: -32001, ValidationError: "Unknown payload"}
 	InvalidTB          = rpc.CustomError{Code: -32002, ValidationError: "Invalid terminal block"}

core/beacon/types.go

@@ -72,18 +72,6 @@ type executableDataMarshaling struct {
 	Transactions  []hexutil.Bytes
 }
 
-type NewBlockResponse struct {
-	Valid bool `json:"valid"`
-}
-
-type GenericResponse struct {
-	Success bool `json:"success"`
-}
-
-type GenericStringResponse struct {
-	Status string `json:"status"`
-}
-
 type ExecutePayloadResponse struct {
 	Status          string      `json:"status"`
 	LatestValidHash common.Hash `json:"latestValidHash"`

eth/catalyst/api.go

@@ -81,7 +81,7 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa
 	log.Trace("Engine API request received", "method", "ForkChoiceUpdated", "head", update.HeadBlockHash, "finalized", update.FinalizedBlockHash, "safe", update.SafeBlockHash)
 	if update.HeadBlockHash == (common.Hash{}) {
 		log.Warn("Forkchoice requested update to zero hash")
-		return beacon.ForkChoiceResponse{Status: beacon.SUCCESS.Status, PayloadID: nil}, nil
+		return beacon.ForkChoiceResponse{Status: beacon.INVALID}, nil // TODO(karalabe): Why does someone send us this?
 	}
 	// Check whether we have the block yet in our database or not. If not, we'll
 	// need to either trigger a sync, or to reject this forkchoice update for a
@@ -95,7 +95,7 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa
 		header := api.remoteBlocks.get(update.HeadBlockHash)
 		if header == nil {
 			log.Warn("Forkcoice requested unknown head", "hash", update.HeadBlockHash)
-			return beacon.INVALID, errors.New("head hash never advertised")
+			return beacon.ForkChoiceResponse{Status: beacon.INVALID}, errors.New("head hash never advertised")
 		}
 		// Header advertised via a past newPayload request. Start syncing to it.
 		// Before we do however, make sure any legacy sync in switched off so we
@@ -106,20 +106,24 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa
 		}
 		log.Info("Forkchoice requested sync to new head", "number", header.Number, "hash", header.Hash())
 		if err := api.eth.Downloader().BeaconSync(api.eth.SyncMode(), header); err != nil {
-			return beacon.ForkChoiceResponse{Status: beacon.SYNCING.Status, PayloadID: nil}, err
+			return beacon.ForkChoiceResponse{Status: beacon.SYNCING}, err
 		}
-		return beacon.ForkChoiceResponse{Status: beacon.SYNCING.Status, PayloadID: nil}, nil
+		return beacon.ForkChoiceResponse{Status: beacon.SYNCING}, nil
+	}
+	// Block is known locally, just sanity check that the beacon client does not
+	// attempt to push as back to before the merge.
+	if block.Difficulty().BitLen() > 0 {
+		log.Error("Refusing beacon update to pre-merge", "number", block.NumberU64(), "hash", update.HeadBlockHash, "diff", block.Difficulty(), "age", common.PrettyAge(time.Unix(int64(block.Time()), 0)))
+		return beacon.ForkChoiceResponse{Status: beacon.INVALID}, errors.New("refusing reorg to pre-merge")
 	}
 	// If the head block is already in our canonical chain, the beacon client is
 	// probably resyncing. Ignore the update.
 	if rawdb.ReadCanonicalHash(api.eth.ChainDb(), block.NumberU64()) == update.HeadBlockHash {
 		log.Warn("Ignoring beacon update to old head", "number", block.NumberU64(), "hash", update.HeadBlockHash, "age", common.PrettyAge(time.Unix(int64(block.Time()), 0)), "have", api.eth.BlockChain().CurrentBlock().NumberU64())
-		return beacon.ForkChoiceResponse{Status: beacon.VALID.Status, PayloadID: nil}, nil
+		return beacon.ForkChoiceResponse{Status: beacon.VALID}, nil
 	}
-	// Requested head is known - and processed locally - but is not canonical.
-	// Either it is a new block on top of our head or a side chain. Reorg.
 	if err := api.eth.BlockChain().SetChainHead(block); err != nil {
-		return beacon.INVALID, err
+		return beacon.ForkChoiceResponse{Status: beacon.INVALID}, err
 	}
 	api.eth.SetSynced()
 
@@ -140,15 +144,15 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa
 		data, err := api.assembleBlock(update.HeadBlockHash, payloadAttributes)
 		if err != nil {
 			log.Error("Failed to create sealing payload", "err", err)
-			return beacon.INVALID, err
+			return beacon.ForkChoiceResponse{Status: beacon.VALID}, err // Valid as setHead was accepted
 		}
 		id := computePayloadId(update.HeadBlockHash, payloadAttributes)
 		api.localBlocks.put(id, data)
 
 		log.Info("Created payload for sealing", "id", id, "elapsed", time.Since(start))
-		return beacon.ForkChoiceResponse{Status: beacon.SUCCESS.Status, PayloadID: &id}, nil
+		return beacon.ForkChoiceResponse{Status: beacon.VALID, PayloadID: &id}, nil
 	}
-	return beacon.ForkChoiceResponse{Status: beacon.SUCCESS.Status, PayloadID: nil}, nil
+	return beacon.ForkChoiceResponse{Status: beacon.VALID}, nil
 }
 
 // GetPayloadV1 returns a cached payload by id.
@@ -172,7 +176,7 @@ func (api *ConsensusAPI) ExecutePayloadV1(params beacon.ExecutableDataV1) (beaco
 	// return a fake success.
 	if block := api.eth.BlockChain().GetBlockByHash(params.BlockHash); block != nil {
 		log.Warn("Ignoring already known beacon payload", "number", params.Number, "hash", params.BlockHash, "age", common.PrettyAge(time.Unix(int64(block.Time()), 0)))
-		return beacon.ExecutePayloadResponse{Status: beacon.VALID.Status, LatestValidHash: block.Hash()}, nil
+		return beacon.ExecutePayloadResponse{Status: beacon.VALID, LatestValidHash: block.Hash()}, nil
 	}
 	// If the parent is missing, we - in theory - could trigger a sync, but that
 	// would also entail a reorg. That is problematic if multiple sibling blocks
@@ -191,15 +195,14 @@ func (api *ConsensusAPI) ExecutePayloadV1(params beacon.ExecutableDataV1) (beaco
 		// some strain from the forkchoice update.
 		if err := api.eth.Downloader().BeaconExtend(api.eth.SyncMode(), block.Header()); err == nil {
 			log.Debug("Payload accepted for sync extension", "number", params.Number, "hash", params.BlockHash)
-			return beacon.ExecutePayloadResponse{Status: beacon.SYNCING.Status, LatestValidHash: api.eth.BlockChain().CurrentBlock().Hash()}, nil
+			return beacon.ExecutePayloadResponse{Status: beacon.SYNCING, LatestValidHash: api.eth.BlockChain().CurrentBlock().Hash()}, nil
 		}
 		// Either no beacon sync was started yet, or it rejected the delivered
 		// payload as non-integratable on top of the existing sync. We'll just
 		// have to rely on the beacon client to forcefully update the head with
 		// a forkchoice update request.
 		log.Warn("Ignoring payload with missing parent", "number", params.Number, "hash", params.BlockHash, "parent", params.ParentHash)
-		// TODO(karalabe): Change this to ACCEPTED once it's included in the code
-		return beacon.ExecutePayloadResponse{Status: beacon.SYNCING.Status, LatestValidHash: common.Hash{}}, nil
+		return beacon.ExecutePayloadResponse{Status: beacon.ACCEPTED, LatestValidHash: common.Hash{}}, nil
 	}
 	// We have an existing parent, do some sanity checks to avoid the beacon client
 	// triggering too early
@@ -222,7 +225,7 @@ func (api *ConsensusAPI) ExecutePayloadV1(params beacon.ExecutableDataV1) (beaco
 		merger.ReachTTD()
 		api.eth.Downloader().Cancel()
 	}
-	return beacon.ExecutePayloadResponse{Status: beacon.VALID.Status, LatestValidHash: block.Hash()}, nil
+	return beacon.ExecutePayloadResponse{Status: beacon.VALID, LatestValidHash: block.Hash()}, nil
 }
 
 // computePayloadId computes a pseudo-random payloadid, based on the parameters.
@@ -240,7 +243,7 @@ func computePayloadId(headBlockHash common.Hash, params *beacon.PayloadAttribute
 
 // invalid returns a response "INVALID" with the latest valid hash set to the current head.
 func (api *ConsensusAPI) invalid() beacon.ExecutePayloadResponse {
-	return beacon.ExecutePayloadResponse{Status: beacon.INVALID.Status, LatestValidHash: api.eth.BlockChain().CurrentHeader().Hash()}
+	return beacon.ExecutePayloadResponse{Status: beacon.INVALID, LatestValidHash: api.eth.BlockChain().CurrentHeader().Hash()}
 }
 
 // assembleBlock creates a new block and returns the "execution

eth/catalyst/api_test.go

@@ -438,7 +438,7 @@ func TestFullAPI(t *testing.T) {
 		if err != nil {
 			t.Fatalf("error preparing payload, err=%v", err)
 		}
-		if resp.Status != beacon.SUCCESS.Status {
+		if resp.Status != beacon.VALID {
 			t.Fatalf("error preparing payload, invalid status: %v", resp.Status)
 		}
 		payloadID := computePayloadId(parent.Hash(), &params)
@@ -450,7 +450,7 @@ func TestFullAPI(t *testing.T) {
 		if err != nil {
 			t.Fatalf("can't execute payload: %v", err)
 		}
-		if execResp.Status != beacon.VALID.Status {
+		if execResp.Status != beacon.VALID {
 			t.Fatalf("invalid status: %v", execResp.Status)
 		}
 		fcState = beacon.ForkchoiceStateV1{

les/catalyst/api.go

@@ -68,30 +68,30 @@ func NewConsensusAPI(les *les.LightEthereum) *ConsensusAPI {
 //      we return an error since block creation is not supported in les mode
 func (api *ConsensusAPI) ForkchoiceUpdatedV1(heads beacon.ForkchoiceStateV1, payloadAttributes *beacon.PayloadAttributesV1) (beacon.ForkChoiceResponse, error) {
 	if heads.HeadBlockHash == (common.Hash{}) {
-		return beacon.ForkChoiceResponse{Status: beacon.SUCCESS.Status, PayloadID: nil}, nil
+		return beacon.ForkChoiceResponse{Status: beacon.VALID}, nil
 	}
 	if err := api.checkTerminalTotalDifficulty(heads.HeadBlockHash); err != nil {
 		if header := api.les.BlockChain().GetHeaderByHash(heads.HeadBlockHash); header == nil {
 			// TODO (MariusVanDerWijden) trigger sync
-			return beacon.SYNCING, nil
+			return beacon.ForkChoiceResponse{Status: beacon.SYNCING}, nil
 		}
-		return beacon.INVALID, err
+		return beacon.ForkChoiceResponse{Status: beacon.INVALID}, err
 	}
 	// If the finalized block is set, check if it is in our blockchain
 	if heads.FinalizedBlockHash != (common.Hash{}) {
 		if header := api.les.BlockChain().GetHeaderByHash(heads.FinalizedBlockHash); header == nil {
 			// TODO (MariusVanDerWijden) trigger sync
-			return beacon.SYNCING, nil
+			return beacon.ForkChoiceResponse{Status: beacon.SYNCING}, nil
 		}
 	}
 	// SetHead
 	if err := api.setHead(heads.HeadBlockHash); err != nil {
-		return beacon.INVALID, err
+		return beacon.ForkChoiceResponse{Status: beacon.INVALID}, err
 	}
 	if payloadAttributes != nil {
-		return beacon.INVALID, errors.New("not supported")
+		return beacon.ForkChoiceResponse{Status: beacon.INVALID}, errors.New("not supported")
 	}
-	return beacon.ForkChoiceResponse{Status: beacon.SUCCESS.Status, PayloadID: nil}, nil
+	return beacon.ForkChoiceResponse{Status: beacon.VALID}, nil
 }
 
 // GetPayloadV1 returns a cached payload by id. It's not supported in les mode.
@@ -113,7 +113,7 @@ func (api *ConsensusAPI) ExecutePayloadV1(params beacon.ExecutableDataV1) (beaco
 			}
 		*/
 		// TODO (MariusVanDerWijden) we should return nil here not empty hash
-		return beacon.ExecutePayloadResponse{Status: beacon.SYNCING.Status, LatestValidHash: common.Hash{}}, nil
+		return beacon.ExecutePayloadResponse{Status: beacon.SYNCING, LatestValidHash: common.Hash{}}, nil
 	}
 	parent := api.les.BlockChain().GetHeaderByHash(params.ParentHash)
 	if parent == nil {
@@ -130,12 +130,12 @@ func (api *ConsensusAPI) ExecutePayloadV1(params beacon.ExecutableDataV1) (beaco
 	if merger := api.les.Merger(); !merger.TDDReached() {
 		merger.ReachTTD()
 	}
-	return beacon.ExecutePayloadResponse{Status: beacon.VALID.Status, LatestValidHash: block.Hash()}, nil
+	return beacon.ExecutePayloadResponse{Status: beacon.VALID, LatestValidHash: block.Hash()}, nil
 }
 
 // invalid returns a response "INVALID" with the latest valid hash set to the current head.
 func (api *ConsensusAPI) invalid() beacon.ExecutePayloadResponse {
-	return beacon.ExecutePayloadResponse{Status: beacon.INVALID.Status, LatestValidHash: api.les.BlockChain().CurrentHeader().Hash()}
+	return beacon.ExecutePayloadResponse{Status: beacon.INVALID, LatestValidHash: api.les.BlockChain().CurrentHeader().Hash()}
 }
 
 func (api *ConsensusAPI) checkTerminalTotalDifficulty(head common.Hash) error {