wire: add proper types for flag field and improve docs

Summary of changes:

- Add a new const TxFlagMarker to indicate the flag prefix byte.
- Add a new TxFlag type to enumerate the flags supported by the
  tx parser.

  This allows us to avoid hardcoded magics, and will make it easier
  to support new flags in future.
- Improve code comments.

Closes #1598.

wire/msgtx.go

@@ -109,14 +109,38 @@ const (
 	maxWitnessItemSize = 11000
 )
 
-// witnessMarkerBytes are a pair of bytes specific to the witness encoding. If
+// TxFlagMarker is the first byte of the FLAG field in a bitcoin tx
-// this sequence is encoutered, then it indicates a transaction has iwtness
+// message. It allows decoders to distinguish a regular serialized
-// data. The first byte is an always 0x00 marker byte, which allows decoders to
+// transaction from one that would require a different parsing logic.
-// distinguish a serialized transaction with witnesses from a regular (legacy)
+//
-// one. The second byte is the Flag field, which at the moment is always 0x01,
+// Position of FLAG in a bitcoin tx message:
-// but may be extended in the future to accommodate auxiliary non-committed
+//   ┌─────────┬────────────────────┬─────────────┬─────┐
-// fields.
+//   │ VERSION │ FLAG               │ TX-IN-COUNT │ ... │
-var witessMarkerBytes = []byte{0x00, 0x01}
+//   │ 4 bytes │ 2 bytes (optional) │ varint      │     │
+//   └─────────┴────────────────────┴─────────────┴─────┘
+//
+// Zooming into the FLAG field:
+//   ┌── FLAG ─────────────┬────────┐
+//   │ TxFlagMarker (0x00) │ TxFlag │
+//   │ 1 byte              │ 1 byte │
+//   └─────────────────────┴────────┘
+const TxFlagMarker = 0x00
+
+// TxFlag is the second byte of the FLAG field in a bitcoin tx message.
+// It indicates the decoding logic to use in the transaction parser, if
+// TxFlagMarker is detected in the tx message.
+//
+// As of writing this, only the witness flag (0x01) is supported, but may be
+// extended in the future to accommodate auxiliary non-committed fields.
+type TxFlag = byte
+
+const (
+	// WitnessFlag is a flag specific to witness encoding. If the TxFlagMarker
+	// is encountered followed by the WitnessFlag, then it indicates a
+	// transaction has witness data. This allows decoders to distinguish a
+	// serialized transaction with witnesses from a legacy one.
+	WitnessFlag TxFlag = 0x01
+)
 
 // scriptFreeList defines a free list of byte slices (up to the maximum number
 // defined by the freeListMaxItems constant) that have a cap according to the
@@ -420,18 +444,19 @@ func (msg *MsgTx) BtcDecode(r io.Reader, pver uint32, enc MessageEncoding) error
 		return err
 	}
 
-	// A count of zero (meaning no TxIn's to the uninitiated) indicates
+	// A count of zero (meaning no TxIn's to the uninitiated) means that the
-	// this is a transaction with witness data.
+	// value is a TxFlagMarker, and hence indicates the presence of a flag.
-	var flag [1]byte
+	var flag [1]TxFlag
-	if count == 0 && enc == WitnessEncoding {
+	if count == TxFlagMarker && enc == WitnessEncoding {
-		// Next, we need to read the flag, which is a single byte.
+		// The count varint was in fact the flag marker byte. Next, we need to
+		// read the flag value, which is a single byte.
 		if _, err = io.ReadFull(r, flag[:]); err != nil {
 			return err
 		}
 
-		// At the moment, the flag MUST be 0x01. In the future other
+		// At the moment, the flag MUST be WitnessFlag (0x01). In the future
-		// flag types may be supported.
+		// other flag types may be supported.
-		if flag[0] != 0x01 {
+		if flag[0] != WitnessFlag {
 			str := fmt.Sprintf("witness tx but flag byte is %x", flag)
 			return messageError("MsgTx.BtcDecode", str)
 		}
@@ -690,14 +715,11 @@ func (msg *MsgTx) BtcEncode(w io.Writer, pver uint32, enc MessageEncoding) error
 	// defined in BIP0144.
 	doWitness := enc == WitnessEncoding && msg.HasWitness()
 	if doWitness {
-		// After the txn's Version field, we include two additional
+		// After the transaction's Version field, we include two additional
-		// bytes specific to the witness encoding. The first byte is an
+		// bytes specific to the witness encoding. This byte sequence is known
-		// always 0x00 marker byte, which allows decoders to
+		// as a flag. The first byte is a marker byte (TxFlagMarker) and the
-		// distinguish a serialized transaction with witnesses from a
+		// second one is the flag value to indicate presence of witness data.
-		// regular (legacy) one. The second byte is the Flag field,
+		if _, err := w.Write([]byte{TxFlagMarker, WitnessFlag}); err != nil {
-		// which at the moment is always 0x01, but may be extended in
-		// the future to accommodate auxiliary non-committed fields.
-		if _, err := w.Write(witessMarkerBytes); err != nil {
 			return err
 		}
 	}

wire/msgtx_test.go

@@ -935,9 +935,9 @@ var multiWitnessTx = &MsgTx{
 // tests.
 var multiWitnessTxEncoded = []byte{
 	0x1, 0x0, 0x0, 0x0, // Version
-	0x0, // Marker byte indicating 0 inputs, or a segwit encoded tx
+	TxFlagMarker, // Marker byte indicating 0 inputs, or a segwit encoded tx
-	0x1, // Flag byte
+	WitnessFlag,  // Flag byte
-	0x1, // Varint for number of inputs
+	0x1,          // Varint for number of inputs
 	0xa5, 0x33, 0x52, 0xd5, 0x13, 0x57, 0x66, 0xf0,
 	0x30, 0x76, 0x59, 0x74, 0x18, 0x26, 0x3d, 0xa2,
 	0xd9, 0xc9, 0x58, 0x31, 0x59, 0x68, 0xfe, 0xa8,
@@ -978,9 +978,9 @@ var multiWitnessTxEncoded = []byte{
 // being set to 0x01, the flag is 0x00, which should trigger a decoding error.
 var multiWitnessTxEncodedNonZeroFlag = []byte{
 	0x1, 0x0, 0x0, 0x0, // Version
-	0x0, // Marker byte indicating 0 inputs, or a segwit encoded tx
+	TxFlagMarker, // Marker byte indicating 0 inputs, or a segwit encoded tx
-	0x0, // Incorrect flag byte (should be 0x01)
+	0x0,          // Incorrect flag byte (should be 0x01)
-	0x1, // Varint for number of inputs
+	0x1,          // Varint for number of inputs
 	0xa5, 0x33, 0x52, 0xd5, 0x13, 0x57, 0x66, 0xf0,
 	0x30, 0x76, 0x59, 0x74, 0x18, 0x26, 0x3d, 0xa2,
 	0xd9, 0xc9, 0x58, 0x31, 0x59, 0x68, 0xfe, 0xa8,