signer, external, api: add clique signing test to debug rpc, fix clique signing in clef

accounts/external/backend.go

@@ -26,7 +26,6 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/event"
 	"github.com/ethereum/go-ethereum/internal/ethapi"
 	"github.com/ethereum/go-ethereum/log"
@@ -154,9 +153,19 @@ func (api *ExternalSigner) signHash(account accounts.Account, hash []byte) ([]by
 
 // SignData signs keccak256(data). The mimetype parameter describes the type of data being signed
 func (api *ExternalSigner) SignData(account accounts.Account, mimeType string, data []byte) ([]byte, error) {
-	// TODO! Replace this with a call to clef SignData with correct mime-type for Clique, once we
+	var res hexutil.Bytes
-	// have that in place
+	var signAddress = common.NewMixedcaseAddress(account.Address)
-	return api.signHash(account, crypto.Keccak256(data))
+	if err := api.client.Call(&res, "account_signData",
+		mimeType,
+		&signAddress, // Need to use the pointer here, because of how MarshalJSON is defined
+		hexutil.Encode(data)); err != nil {
+		return nil, err
+	}
+	// If V is on 27/28-form, convert to to 0/1 for Clique
+	if mimeType == accounts.MimetypeClique && (res[64] == 27 || res[64] == 28) {
+		res[64] -= 27 // Transform V from 27/28 to 0/1 for Clique use
+	}
+	return res, nil
 }
 
 func (api *ExternalSigner) SignText(account accounts.Account, text []byte) ([]byte, error) {
@@ -210,18 +219,6 @@ func (api *ExternalSigner) listAccounts() ([]common.Address, error) {
 	return res, nil
 }
 
-func (api *ExternalSigner) signCliqueBlock(a common.Address, rlpBlock hexutil.Bytes) (hexutil.Bytes, error) {
-	var sig hexutil.Bytes
-	if err := api.client.Call(&sig, "account_signData", core.ApplicationClique.Mime, a, rlpBlock); err != nil {
-		return nil, err
-	}
-	if sig[64] != 27 && sig[64] != 28 {
-		return nil, fmt.Errorf("invalid Ethereum signature (V is not 27 or 28)")
-	}
-	sig[64] -= 27 // Transform V from 27/28 to 0/1 for Clique use
-	return sig, nil
-}
-
 func (api *ExternalSigner) pingVersion() (string, error) {
 	var v string
 	if err := api.client.Call(&v, "account_version"); err != nil {

cmd/clef/tests/testsigner.js

@@ -1,4 +1,9 @@
 // This file is a test-utility for testing clef-functionality
+//
+// Start clef with
+//
+// build/bin/clef --4bytedb=./cmd/clef/4byte.json --rpc
+//
 // Start geth with
 //
 // build/bin/geth --nodiscover --maxpeers 0 --signer http://localhost:8550 console --preload=cmd/clef/tests/testsigner.js
@@ -12,9 +17,12 @@
 function reload(){
 	loadScript("./cmd/clef/tests/testsigner.js");
 }
+
 function init(){
-    accts = eth.accounts
+    if (typeof accts == 'undefined' || accts.length == 0){
-    console.log("Got accounts ", accts);
+        accts = eth.accounts
+        console.log("Got accounts ", accts);
+    }
 }
 init()
 function testTx(){
@@ -30,17 +38,26 @@ function testSignText(){
         var r = eth.sign(a, "0x68656c6c6f20776f726c64"); //hello world
         console.log("signing response",  r)
     }
-
 }
-function test(){
+function testClique(){
-	try{
+    if( accts && accts.length > 0){
-		testTx()
+        var a = accts[0]
-    }catch(err){
+        var r = debug.testSignCliqueBlock(a, 0); // Sign genesis
-		console.log(err)
+        console.log("signing response",  r)
-	}
-    try{
-        testSignText()
-    }catch(err){
-        console.log(err)
     }
 }
+
+function test(){
+    var tests = [
+        testTx,
+        testSignText,
+        testSignClique,
+    ]
+    for( i in tests){
+        try{
+            tests[i]()
+        }catch(err){
+            console.log(err)
+        }
+    }
+ }

internal/ethapi/api.go

@@ -31,6 +31,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/common/math"
+	"github.com/ethereum/go-ethereum/consensus/clique"
 	"github.com/ethereum/go-ethereum/consensus/ethash"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/rawdb"
@@ -1481,6 +1482,44 @@ func (api *PublicDebugAPI) GetBlockRlp(ctx context.Context, number uint64) (stri
 	return fmt.Sprintf("%x", encoded), nil
 }
 
+// TestSignCliqueBlock fetches the given block number, and attempts to sign it as a clique header with the
+// given address, returning the address of the recovered signature
+func (api *PublicDebugAPI) TestSignCliqueBlock(ctx context.Context, address common.Address, number uint64) (common.Address, error) {
+	block, _ := api.b.BlockByNumber(ctx, rpc.BlockNumber(number))
+	if block == nil {
+		return common.Address{}, fmt.Errorf("block #%d not found", number)
+	}
+	header := block.Header()
+	header.Extra = make([]byte, 65)
+	encoded, err := rlp.EncodeToBytes(header)
+	if err != nil {
+		return common.Address{}, err
+	}
+	// Look up the wallet containing the requested signer
+	account := accounts.Account{Address: address}
+	wallet, err := api.b.AccountManager().Find(account)
+	if err != nil {
+		return common.Address{}, err
+	}
+
+	signature, err := wallet.SignData(account, accounts.MimetypeClique, encoded)
+	if err != nil {
+		return common.Address{}, err
+	}
+	sealHash := clique.SealHash(header).Bytes()
+	log.Info("test signing of clique block",
+		"Sealhash", fmt.Sprintf("%x", sealHash),
+		"signature", fmt.Sprintf("%x", signature))
+	pubkey, err := crypto.Ecrecover(sealHash, signature)
+	if err != nil {
+		return common.Address{}, err
+	}
+	var signer common.Address
+	copy(signer[:], crypto.Keccak256(pubkey[1:])[12:])
+
+	return signer, nil
+}
+
 // PrintBlock retrieves a block and returns its pretty printed form.
 func (api *PublicDebugAPI) PrintBlock(ctx context.Context, number uint64) (string, error) {
 	block, _ := api.b.BlockByNumber(ctx, rpc.BlockNumber(number))

internal/web3ext/web3ext.go

@@ -231,6 +231,12 @@ web3._extend({
 			call: 'debug_getBlockRlp',
 			params: 1
 		}),
+		new web3._extend.Method({
+			name: 'testSignCliqueBlock',
+			call: 'debug_testSignCliqueBlock',
+			params: 2, 
+			inputFormatters: [web3._extend.formatters.inputAddressFormatter, null],
+		}),
 		new web3._extend.Method({
 			name: 'setHead',
 			call: 'debug_setHead',

signer/core/signed_data.go

@@ -121,8 +121,8 @@ var typedDataReferenceTypeRegexp = regexp.MustCompile(`^[A-Z](\w*)(\[\])?$`)
 // sign receives a request and produces a signature
 
 // Note, the produced signature conforms to the secp256k1 curve R, S and V values,
-// where the V value will be 27 or 28 for legacy reasons.
+// where the V value will be 27 or 28 for legacy reasons, if legacyV==true.
-func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest) (hexutil.Bytes, error) {
+func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest, legacyV bool) (hexutil.Bytes, error) {
 
 	// We make the request prior to looking up if we actually have the account, to prevent
 	// account-enumeration via the API
@@ -144,7 +144,9 @@ func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest) (
 	if err != nil {
 		return nil, err
 	}
-	signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper
+	if legacyV {
+		signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper
+	}
 	return signature, nil
 }
 
@@ -153,17 +155,16 @@ func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest) (
 //
 // Different types of validation occur.
 func (api *SignerAPI) SignData(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (hexutil.Bytes, error) {
-	var req, err = api.determineSignatureFormat(ctx, contentType, addr, data)
+	var req, transformV, err = api.determineSignatureFormat(ctx, contentType, addr, data)
 	if err != nil {
 		return nil, err
 	}
 
-	signature, err := api.sign(addr, req)
+	signature, err := api.sign(addr, req, transformV)
 	if err != nil {
 		api.UI.ShowError(err.Error())
 		return nil, err
 	}
-
 	return signature, nil
 }
 
@@ -173,12 +174,14 @@ func (api *SignerAPI) SignData(ctx context.Context, contentType string, addr com
 // charset, ok := params["charset"]
 // As it is now, we accept any charset and just treat it as 'raw'.
 // This method returns the mimetype for signing along with the request
-func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (*SignDataRequest, error) {
+func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (*SignDataRequest, bool, error) {
-	var req *SignDataRequest
+	var (
-
+		req        *SignDataRequest
+		useLegacyV = true // Default to use V = 27 or 28, the legacy Ethereum format
+	)
 	mediaType, _, err := mime.ParseMediaType(contentType)
 	if err != nil {
-		return nil, err
+		return nil, useLegacyV, err
 	}
 
 	switch mediaType {
@@ -186,7 +189,7 @@ func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType
 		// Data with an intended validator
 		validatorData, err := UnmarshalValidatorData(data)
 		if err != nil {
-			return nil, err
+			return nil, useLegacyV, err
 		}
 		sighash, msg := SignTextValidator(validatorData)
 		message := []*NameValueType{
@@ -201,39 +204,40 @@ func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType
 		// Clique is the Ethereum PoA standard
 		stringData, ok := data.(string)
 		if !ok {
-			return nil, fmt.Errorf("input for %v plain must be an hex-encoded string", ApplicationClique.Mime)
+			return nil, useLegacyV, fmt.Errorf("input for %v must be an hex-encoded string", ApplicationClique.Mime)
 		}
 		cliqueData, err := hexutil.Decode(stringData)
 		if err != nil {
-			return nil, err
+			return nil, useLegacyV, err
 		}
 		header := &types.Header{}
 		if err := rlp.DecodeBytes(cliqueData, header); err != nil {
-			return nil, err
+			return nil, useLegacyV, err
 		}
 		// Get back the rlp data, encoded by us
-		cliqueData = clique.CliqueRLP(header)
+		sighash, cliqueRlp, err := cliqueHeaderHashAndRlp(header)
-		sighash, err := SignCliqueHeader(header)
 		if err != nil {
-			return nil, err
+			return nil, useLegacyV, err
 		}
 		message := []*NameValueType{
 			{
-				Name:  "Clique block",
+				Name:  "Clique header",
 				Typ:   "clique",
-				Value: fmt.Sprintf("clique block %d [0x%x]", header.Number, header.Hash()),
+				Value: fmt.Sprintf("clique header %d [0x%x]", header.Number, header.Hash()),
 			},
 		}
-		req = &SignDataRequest{ContentType: mediaType, Rawdata: cliqueData, Message: message, Hash: sighash}
+		// Clique uses V on the form 0 or 1
+		useLegacyV = false
+		req = &SignDataRequest{ContentType: mediaType, Rawdata: cliqueRlp, Message: message, Hash: sighash}
 	default: // also case TextPlain.Mime:
 		// Calculates an Ethereum ECDSA signature for:
 		// hash = keccak256("\x19${byteVersion}Ethereum Signed Message:\n${message length}${message}")
 		// We expect it to be a string
 		if stringData, ok := data.(string); !ok {
-			return nil, fmt.Errorf("input for text/plain must be an hex-encoded string")
+			return nil, useLegacyV, fmt.Errorf("input for text/plain must be an hex-encoded string")
 		} else {
 			if textData, err := hexutil.Decode(stringData); err != nil {
-				return nil, err
+				return nil, useLegacyV, err
 			} else {
 				sighash, msg := accounts.TextAndHash(textData)
 				message := []*NameValueType{
@@ -249,7 +253,7 @@ func (api *SignerAPI) determineSignatureFormat(ctx context.Context, contentType
 	}
 	req.Address = addr
 	req.Meta = MetadataFromContext(ctx)
-	return req, nil
+	return req, useLegacyV, nil
 
 }
 
@@ -262,20 +266,21 @@ func SignTextValidator(validatorData ValidatorData) (hexutil.Bytes, string) {
 	return crypto.Keccak256([]byte(msg)), msg
 }
 
-// SignCliqueHeader returns the hash which is used as input for the proof-of-authority
+// cliqueHeaderHashAndRlp returns the hash which is used as input for the proof-of-authority
 // signing. It is the hash of the entire header apart from the 65 byte signature
 // contained at the end of the extra data.
 //
 // The method requires the extra data to be at least 65 bytes -- the original implementation
 // in clique.go panics if this is the case, thus it's been reimplemented here to avoid the panic
 // and simply return an error instead
-func SignCliqueHeader(header *types.Header) (hexutil.Bytes, error) {
+func cliqueHeaderHashAndRlp(header *types.Header) (hash, rlp []byte, err error) {
-	//hash := common.Hash{}
 	if len(header.Extra) < 65 {
-		return nil, fmt.Errorf("clique header extradata too short, %d < 65", len(header.Extra))
+		err = fmt.Errorf("clique header extradata too short, %d < 65", len(header.Extra))
+		return
 	}
-	hash := clique.SealHash(header)
+	rlp = clique.CliqueRLP(header)
-	return hash.Bytes(), nil
+	hash = clique.SealHash(header).Bytes()
+	return hash, rlp, err
 }
 
 // SignTypedData signs EIP-712 conformant typed data
@@ -293,7 +298,7 @@ func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAd
 	sighash := crypto.Keccak256(rawData)
 	message := typedData.Format()
 	req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: rawData, Message: message, Hash: sighash}
-	signature, err := api.sign(addr, req)
+	signature, err := api.sign(addr, req, true)
 	if err != nil {
 		api.UI.ShowError(err.Error())
 		return nil, err