preprocess_key(), parse_wif(): fixes and cleanups

- wif2hex() -> parse_wif()
- reimplement both methods using bytes instead of hex strings
- fix bug with zcash-t preprocess_key() failing to call base class method,
  causing crash with edge-case privkeys

mmgen/addr.py

@@ -204,7 +204,7 @@ class AddrGeneratorMonero(AddrGenerator):
 
 	def to_viewkey(self,sk_hex):
 		assert len(sk_hex) == 64,'{}: incorrect privkey length'.format(len(sk_hex))
-		return MoneroViewKey(g.proto.preprocess_key(self.keccak_256(bytes.fromhex(sk_hex)).hexdigest(),None))
+		return MoneroViewKey(g.proto.preprocess_key(self.keccak_256(bytes.fromhex(sk_hex)).digest(),None).hex())
 
 	def to_segwit_redeem_script(self,sk_hex):
 		raise NotImplementedError('Monero addresses incompatible with Segwit')

mmgen/exception.py

@@ -48,6 +48,7 @@ class MaxInputSizeExceeded(Exception):    mmcode = 3
 class WalletFileError(Exception):         mmcode = 3
 class HexadecimalStringError(Exception):  mmcode = 3
 class SeedLengthError(Exception):         mmcode = 3
+class PrivateKeyError(Exception):         mmcode = 3
 
 # 4: red hl, 'MMGen Fatal Error' + exception + message
 class BadMMGenTxID(Exception):            mmcode = 4

mmgen/obj.py

@@ -689,6 +689,10 @@ class MoneroViewKey(HexStr):  color,width,hexcase = 'cyan',64,'lower'
 class MMGenTxID(HexStr):      color,width,hexcase = 'red',6,'upper'
 
 class WifKey(str,Hilite,InitErrors):
+	"""
+	Initialize a WIF key, checking its well-formedness.
+	The numeric validity of the private key it encodes is not checked.
+	"""
 	width = 53
 	color = 'blue'
 	def __new__(cls,s,on_fail='die'):
@@ -697,7 +701,7 @@ class WifKey(str,Hilite,InitErrors):
 		try:
 			assert set(s) <= set(ascii_letters+digits),'not an ascii alphanumeric string'
 			from mmgen.globalvars import g
-			g.proto.wif2hex(s) # raises exception on error
+			g.proto.parse_wif(s) # raises exception on error
 			return str.__new__(cls,s)
 		except Exception as e:
 			return cls.init_fail(e,s)
@@ -714,7 +718,12 @@ class PubKey(HexStr,MMGenObject): # TODO: add some real checks
 			return me
 
 class PrivKey(str,Hilite,InitErrors,MMGenObject):
-
+	"""
+	Input:   a) raw, non-preprocessed bytes; or b) WIF key.
+	Output:  preprocessed hexadecimal key, plus WIF key in 'wif' attribute
+	For coins without a WIF format, 'wif' contains the preprocessed hex.
+	The numeric validity of the resulting key is always checked.
+	"""
 	color = 'red'
 	width = 64
 	trunc_ok = False
@@ -733,18 +742,21 @@ class PrivKey(str,Hilite,InitErrors,MMGenObject):
 			try:
 				assert s == None,"'wif' and key hex args are mutually exclusive"
 				assert set(wif) <= set(ascii_letters+digits),'not an ascii alphanumeric string'
-				w2h = g.proto.wif2hex(wif) # raises exception on error
-				me = str.__new__(cls,w2h['hex'])
-				me.compressed = w2h['compressed']
-				me.pubkey_type = w2h['pubkey_type']
+				k = g.proto.parse_wif(wif) # raises exception on error
+				me = str.__new__(cls,k.sec.hex())
+				me.compressed = k.compressed
+				me.pubkey_type = k.pubkey_type
 				me.wif = str.__new__(WifKey,wif) # check has been done
 				me.orig_hex = None
+				if k.sec != g.proto.preprocess_key(k.sec,k.pubkey_type):
+					m = '{} WIF key {!r} encodes private key with unacceptable value {}'
+					raise PrivateKeyError(m.format(g.proto.__name__,me.wif,me))
 				return me
 			except Exception as e:
 				return cls.init_fail(e,s,objname='{} WIF key'.format(g.coin))
 		else:
 			try:
-				assert s,'private key bin data missing'
+				assert s,'private key bytes data missing'
 				assert pubkey_type is not None,"'pubkey_type' arg missing"
 				assert len(s) == cls.width // 2,'key length must be {}'.format(cls.width // 2)
 				if pubkey_type == 'password': # skip WIF creation and pre-processing for passwds
@@ -752,7 +764,7 @@ class PrivKey(str,Hilite,InitErrors,MMGenObject):
 				else:
 					assert compressed is not None, "'compressed' arg missing"
 					assert type(compressed) == bool,"{!r}: 'compressed' not of type 'bool'".format(compressed)
-					me = str.__new__(cls,g.proto.preprocess_key(s.hex(),pubkey_type))
+					me = str.__new__(cls,g.proto.preprocess_key(s,pubkey_type).hex())
 					me.wif = WifKey(g.proto.hex2wif(me,pubkey_type,compressed),on_fail='raise')
 					me.compressed = compressed
 				me.pubkey_type = pubkey_type

mmgen/protocol.py

@@ -21,11 +21,15 @@ protocol.py: Coin protocol functions, classes and methods
 """
 
 import sys,os,hashlib
+from collections import namedtuple
+
 from mmgen.util import msg,ymsg,Msg,ydie
 from mmgen.obj import MMGenObject,BTCAmt,LTCAmt,BCHAmt,B2XAmt,ETHAmt
 from mmgen.globalvars import g
 import mmgen.bech32 as bech32
 
+parsed_wif = namedtuple('parsed_wif',['sec','pubkey_type','compressed'])
+
 def hash160(hexnum): # take hex, return hex - OP_HASH160
 	return hashlib.new('ripemd160',hashlib.sha256(bytes.fromhex(hexnum)).digest()).hexdigest()
 
@@ -108,19 +112,19 @@ class BitcoinProtocol(MMGenObject):
 	def cap(cls,s): return s in cls.caps
 
 	@classmethod
-	def preprocess_key(cls,hexpriv,pubkey_type):
+	def preprocess_key(cls,sec,pubkey_type):
 		# Key must be non-zero and less than group order of secp256k1 curve
-		if 0 < int(hexpriv,16) < cls.secp256k1_ge:
-			return hexpriv
+		if 0 < int.from_bytes(sec,'big') < cls.secp256k1_ge:
+			return sec
 		else: # chance of this is less than 1 in 2^127
-			pk = int(hexpriv,16)
+			pk = int.from_bytes(sec,'big')
 			if pk == 0: # chance of this is 1 in 2^256
 				ydie(3,'Private key is zero!')
 			elif pk == cls.secp256k1_ge: # ditto
 				ydie(3,'Private key == secp256k1_ge!')
 			else:
-				ymsg('Warning: private key is greater than secp256k1 group order!:\n  {}'.format(hexpriv))
-				return '{:064x}'.format(pk % cls.secp256k1_ge).encode()
+				ymsg('Warning: private key is greater than secp256k1 group order!:\n  {}'.format(sec.hex()))
+				return (pk % cls.secp256k1_ge).to_bytes(cls.privkey_len,'big')
 
 	@classmethod
 	def hex2wif(cls,hexpriv,pubkey_type,compressed): # PrivKey
@@ -129,7 +133,7 @@ class BitcoinProtocol(MMGenObject):
 		return _b58chk_encode(cls.wif_ver_num[pubkey_type] + hexpriv + ('','01')[bool(compressed)])
 
 	@classmethod
-	def wif2hex(cls,wif):
+	def parse_wif(cls,wif):
 		key = _b58chk_decode(wif)
 		pubkey_type = None
 		for k,v in list(cls.wif_ver_num.items()):
@@ -143,9 +147,8 @@ class BitcoinProtocol(MMGenObject):
 		else:
 			assert len(key) == 64,'invalid key length'
 			compressed = False
-		assert 0 < int(key[:64],16) < cls.secp256k1_ge,(
-			"'{}': invalid WIF (produces key outside allowable range)".format(wif))
-		return { 'hex':key[:64], 'pubkey_type':pubkey_type, 'compressed':compressed }
+
+		return parsed_wif(bytes.fromhex(key[:64]), pubkey_type, compressed)
 
 	@classmethod
 	def verify_addr(cls,addr,hex_width,return_dict=False):
@@ -296,8 +299,8 @@ class DummyWIF(object):
 		return hexpriv
 
 	@classmethod
-	def wif2hex(cls,wif):
-		return { 'hex':wif, 'pubkey_type':cls.pubkey_type, 'compressed':False }
+	def parse_wif(cls,wif):
+		return parsed_wif(bytes.fromhex(wif), cls.pubkey_type, False)
 
 class EthereumProtocol(DummyWIF,BitcoinProtocol):
 
@@ -362,11 +365,11 @@ class ZcashProtocol(BitcoinProtocolAddrgen):
 	dfl_mmtype   = 'L'
 
 	@classmethod
-	def preprocess_key(cls,hexpriv,pubkey_type): # zero the first four bits
-		if pubkey_type == 'zcash_z':
-			return '{:02x}'.format(int(hexpriv[:2],16) & 0x0f) + hexpriv[2:]
+	def preprocess_key(cls,sec,pubkey_type):
+		if pubkey_type == 'zcash_z': # zero the first four bits
+			return bytes([sec[0] & 0x0f]) + sec[1:]
 		else:
-			return hexpriv
+			return super(cls,cls).preprocess_key(sec,pubkey_type)
 
 	@classmethod
 	def pubhash2addr(cls,pubkey_hash,p2sh):
@@ -398,10 +401,10 @@ class MoneroProtocol(DummyWIF,BitcoinProtocolAddrgen):
 	pubkey_type = 'monero' # required by DummyWIF
 
 	@classmethod
-	def preprocess_key(cls,hexpriv,pubkey_type): # reduce key
+	def preprocess_key(cls,sec,pubkey_type): # reduce key
 		from mmgen.ed25519 import l
-		n = int(bytes.fromhex(hexpriv)[::-1].hex(),16) % l
-		return bytes.fromhex('{:064x}'.format(n))[::-1].hex()
+		n = int.from_bytes(sec[::-1],'big') % l
+		return int.to_bytes(n,cls.privkey_len,'big')[::-1]
 
 	@classmethod
 	def verify_addr(cls,addr,hex_width,return_dict=False):