Merge pull request #895 from marioevz/fixes-from-meeting

simulators/ethereum/engine: Fixes from interop meeting

simulators/ethereum/engine/helper/customizer.go

@@ -728,17 +728,24 @@ func GenerateInvalidPayload(basePayload *typ.ExecutableData, payloadField Invali
 		var customTxData CustomTransactionData
 		switch payloadField {
 		case InvalidTransactionSignature:
-			modifiedSignature := SignatureValuesFromRaw(baseTx.RawSignatureValues())
-			modifiedSignature.R = modifiedSignature.R.Sub(modifiedSignature.R, common.Big1)
+			baseV, baseR, baseS := baseTx.RawSignatureValues()
+			modifiedSignature := SignatureValuesFromRaw(baseV, baseR, baseS)
+			modifiedSignature.S = modifiedSignature.S.Sub(modifiedSignature.S, common.Big1)
 			customTxData.Signature = &modifiedSignature
 		case InvalidTransactionNonce:
-			customNonce := baseTx.Nonce() - 1
+			customNonce := baseTx.Nonce()
+			if customNonce > 0 {
+				customNonce = customNonce - 1
+			} else {
+				customNonce = 2 << 32
+			}
 			customTxData.Nonce = &customNonce
 		case InvalidTransactionGas:
-			customGas := uint64(0)
+			customGas := uint64(baseTx.Gas() + 1)
 			customTxData.Gas = &customGas
 		case InvalidTransactionGasPrice:
-			customTxData.GasPriceOrGasFeeCap = common.Big0
+			customTxData.GasPriceOrGasFeeCap = new(big.Int).Set(baseTx.GasPrice())
+			customTxData.GasPriceOrGasFeeCap.Div(customTxData.GasPriceOrGasFeeCap, big.NewInt(2))
 		case InvalidTransactionGasTipPrice:
 			invalidGasTip := new(big.Int).Set(globals.GasTipPrice)
 			invalidGasTip.Mul(invalidGasTip, big.NewInt(2))

simulators/ethereum/engine/helper/tx.go

@@ -31,9 +31,9 @@ type SignatureValues struct {
 
 func SignatureValuesFromRaw(v *big.Int, r *big.Int, s *big.Int) SignatureValues {
 	return SignatureValues{
-		V: v,
-		R: r,
-		S: s,
+		V: new(big.Int).Set(v),
+		R: new(big.Int).Set(r),
+		S: new(big.Int).Set(s),
 	}
 }
 
@@ -104,6 +104,11 @@ func CustomizeTransaction(baseTransaction *types.Transaction, sender SenderAccou
 	case types.DynamicFeeTxType:
 		modifiedDynamicFeeTxBase := &types.DynamicFeeTx{}
 
+		if customData.ChainID != nil {
+			modifiedDynamicFeeTxBase.ChainID = customData.ChainID
+		} else {
+			modifiedDynamicFeeTxBase.ChainID = baseTransaction.ChainId()
+		}
 		if customData.Nonce != nil {
 			modifiedDynamicFeeTxBase.Nonce = *customData.Nonce
 		} else {
@@ -148,7 +153,11 @@ func CustomizeTransaction(baseTransaction *types.Transaction, sender SenderAccou
 		modifiedTxData = modifiedDynamicFeeTxBase
 	case types.BlobTxType:
 		modifiedBlobTxBase := &types.BlobTx{}
-
+		if customData.ChainID != nil {
+			modifiedBlobTxBase.ChainID = uint256.MustFromBig(customData.ChainID)
+		} else {
+			modifiedBlobTxBase.ChainID = uint256.MustFromBig(baseTransaction.ChainId())
+		}
 		if customData.Nonce != nil {
 			modifiedBlobTxBase.Nonce = *customData.Nonce
 		} else {
@@ -206,7 +215,7 @@ func CustomizeTransaction(baseTransaction *types.Transaction, sender SenderAccou
 		// If a custom invalid signature was not specified, simply sign the transaction again
 		if customData.ChainID == nil {
 			// Use the default value if an invaild chain ID was not specified
-			customData.ChainID = globals.ChainID
+			customData.ChainID = baseTransaction.ChainId()
 		}
 		signer := types.NewCancunSigner(customData.ChainID)
 		var err error

simulators/ethereum/engine/suites/cancun/tests.go

@@ -1061,14 +1061,8 @@ var Tests = []test.Spec{
 		},
 
 		TestSequence: TestSequence{
-			// First, we send a couple of blob transactions on genesis,
-			// with enough data gas cost to make sure they are included in the first block.
-			SendBlobTransactions{
-				TransactionCount:              cancun.TARGET_BLOBS_PER_BLOCK,
-				BlobTransactionMaxBlobGasCost: big.NewInt(1),
-			},
 			NewPayloads{
-				ExpectedIncludedBlobCount: cancun.TARGET_BLOBS_PER_BLOCK,
+				ExpectedIncludedBlobCount: 0,
 				// This customizer only simulates requesting a Shanghai payload 1 second before cancun.
 				// CL Mock will still request the Cancun payload afterwards
 				FcUOnPayloadRequest: &helper.BaseForkchoiceUpdatedCustomizer{

simulators/ethereum/engine/suites/engine/invalid_payload.go

@@ -28,7 +28,8 @@ type InvalidPayloadTestCase struct {
 	Syncing bool
 	// EmptyTransactions is true if the payload should not contain any transactions
 	EmptyTransactions bool
-	// If true, the payload can be detected to be invalid even when syncing
+	// If true, the payload can be detected to be invalid even when syncing,
+	// but this check is optional and both `INVALID` and `SYNCING` are valid responses.
 	InvalidDetectedOnSync bool
 	// If true, latest valid hash can be nil for this test.
 	NilLatestValidHash bool
@@ -153,8 +154,11 @@ func (tc InvalidPayloadTestCase) Execute(t *test.Env) {
 				// if the payload extends the canonical chain and requisite data for its validation is missing
 				// (the client can assume the payload extends the canonical because the linking payload could be missing)
 				if invalidDetectedOnSync {
-					// For some fields, the client can detect the invalid payload even when it doesn't have the parent
-					r.ExpectStatusEither(test.Invalid)
+					// For some fields, the client can detect the invalid payload even when it doesn't have the parent.
+					// However this behavior is up to the client, so we can't expect it to happen and syncing is also valid.
+					// `VALID` response is still incorrect though.
+					r.ExpectStatusEither(test.Invalid, test.Accepted, test.Syncing)
+					// TODO: It seems like latestValidHash==nil should always be expected here.
 				} else {
 					r.ExpectStatusEither(test.Accepted, test.Syncing)
 					r.ExpectLatestValidHash(nil)

simulators/ethereum/engine/suites/engine/tests.go

@@ -195,24 +195,27 @@ func init() {
 		helper.InvalidTransactionValue,
 		helper.InvalidTransactionChainID,
 	} {
+		invalidDetectedOnSync := invalidField == helper.InvalidTransactionChainID
 		for _, syncing := range []bool{false, true} {
 			if invalidField != helper.InvalidTransactionGasTipPrice {
 				for _, testTxType := range []helper.TestTransactionType{helper.LegacyTxOnly, helper.DynamicFeeTxOnly} {
 					Tests = append(Tests, InvalidPayloadTestCase{
 						BaseSpec: test.BaseSpec{
 							TestTransactionType: testTxType,
 						},
-						InvalidField: invalidField,
-						Syncing:      syncing,
+						InvalidField:          invalidField,
+						Syncing:               syncing,
+						InvalidDetectedOnSync: invalidDetectedOnSync,
 					})
 				}
 			} else {
 				Tests = append(Tests, InvalidPayloadTestCase{
 					BaseSpec: test.BaseSpec{
 						TestTransactionType: helper.DynamicFeeTxOnly,
 					},
-					InvalidField: invalidField,
-					Syncing:      syncing,
+					InvalidField:          invalidField,
+					Syncing:               syncing,
+					InvalidDetectedOnSync: invalidDetectedOnSync,
 				})
 			}
 		}