Cleanup a few nitpicks with recent alert additions.

- Group the new read/writeVarBytes functions together to be consistent
  with the existing code
- Modify the comments on the new read/writeVarBytes to be a little more
  descriptive and consistent with existing code
- Use "test payload" for field name in the tests for the
  read/writeVarBytes functions which is more accurate
- Remove reserved param from NewAlert since there is no point is having
  the caller deal with a reserved param
- Various comment tweaks for clarity and consistency
- Use camel case for fuction params for consistency
- Move the NewAlert and NewAlertFromPayload functions after the receiver
  definitions for code layout consistency

Closes #11.

common.go

@@ -281,32 +281,6 @@ func writeElements(w io.Writer, elements ...interface{}) error {
 	return nil
 }
 
-// readVarBytes reads a variable length byte array
-func readVarBytes(r io.Reader, pver uint32, maxAllowed uint32,
-	fieldName string) ([]byte, error) {
-
-	count, err := readVarInt(r, pver)
-	if err != nil {
-		return nil, err
-	}
-
-	// Prevent byte array larger than the max message size.  It would
-	// be possible to cause memory exhaustion and panics without a sane
-	// upper bound on this count.
-	if count > uint64(maxAllowed) {
-		str := fmt.Sprintf("%s is larger than the max allowed size "+
-			"[count %d, max %d]", fieldName, count, maxAllowed)
-		return nil, messageError("readVarBytes", str)
-	}
-
-	b := make([]byte, count)
-	_, err = io.ReadFull(r, b)
-	if err != nil {
-		return nil, err
-	}
-	return b, nil
-}
-
 // readVarInt reads a variable length integer from r and returns it as a uint64.
 func readVarInt(r io.Reader, pver uint32) (uint64, error) {
 	var b [8]byte
@@ -346,21 +320,6 @@ func readVarInt(r io.Reader, pver uint32) (uint64, error) {
 	return rv, nil
 }
 
-// writeVarBytes writes a variable length byte array
-func writeVarBytes(w io.Writer, pver uint32, bytes []byte) error {
-	slen := uint64(len(bytes))
-	err := writeVarInt(w, pver, slen)
-	if err != nil {
-		return err
-	}
-
-	_, err = w.Write(bytes)
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 // writeVarInt serializes val to w using a variable number of bytes depending
 // on its value.
 func writeVarInt(w io.Writer, pver uint32, val uint64) error {
@@ -420,7 +379,7 @@ func VarIntSerializeSize(val uint64) int {
 // string, and the bytes that represent the string itself.  An error is returned
 // if the length is greater than the maximum block payload size, since it would
 // not be possible to put a varString of that size into a block anyways and it
-// also helps protect against memory exhuastion attacks and forced panics
+// also helps protect against memory exhaustion attacks and forced panics
 // through malformed messages.
 func readVarString(r io.Reader, pver uint32) (string, error) {
 	count, err := readVarInt(r, pver)
@@ -459,6 +418,54 @@ func writeVarString(w io.Writer, pver uint32, str string) error {
 	return nil
 }
 
+// readVarBytes reads a variable length byte array.  A byte array is encoded
+// as a varInt containing the length of the array followed by the bytes
+// themselves.  An error is returned if the length is greater than the
+// passed maxAllowed parameter which helps protect against memory exhuastion
+// attacks and forced panics thorugh malformed messages.  The fieldName
+// parameter is only used for the error message so it provides more context in
+// the error.
+func readVarBytes(r io.Reader, pver uint32, maxAllowed uint32,
+	fieldName string) ([]byte, error) {
+
+	count, err := readVarInt(r, pver)
+	if err != nil {
+		return nil, err
+	}
+
+	// Prevent byte array larger than the max message size.  It would
+	// be possible to cause memory exhaustion and panics without a sane
+	// upper bound on this count.
+	if count > uint64(maxAllowed) {
+		str := fmt.Sprintf("%s is larger than the max allowed size "+
+			"[count %d, max %d]", fieldName, count, maxAllowed)
+		return nil, messageError("readVarBytes", str)
+	}
+
+	b := make([]byte, count)
+	_, err = io.ReadFull(r, b)
+	if err != nil {
+		return nil, err
+	}
+	return b, nil
+}
+
+// writeVarInt serializes a variable length byte array to w as a varInt
+// containing the number of bytes, followed by the bytes themselves.
+func writeVarBytes(w io.Writer, pver uint32, bytes []byte) error {
+	slen := uint64(len(bytes))
+	err := writeVarInt(w, pver, slen)
+	if err != nil {
+		return err
+	}
+
+	_, err = w.Write(bytes)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 // randomUint64 returns a cryptographically random uint64 value.  This
 // unexported version takes a reader primarily to ensure the error paths
 // can be properly tested by passing a fake reader in the tests.

common_test.go

@@ -539,8 +539,8 @@ func TestVarBytesWire(t *testing.T) {
 
 		// Decode from wire format.
 		rbuf := bytes.NewBuffer(test.buf)
-		val, err := btcwire.TstReadVarBytes(rbuf, test.pver, btcwire.MaxMessagePayload,
+		val, err := btcwire.TstReadVarBytes(rbuf, test.pver,
-			"alert serialized payload")
+			btcwire.MaxMessagePayload, "test payload")
 		if err != nil {
 			t.Errorf("readVarBytes #%d error %v", i, err)
 			continue
@@ -591,8 +591,8 @@ func TestVarBytesWireErrors(t *testing.T) {
 
 		// Decode from wire format.
 		r := newFixedReader(test.max, test.buf)
-		_, err = btcwire.TstReadVarBytes(r, test.pver, btcwire.MaxMessagePayload,
+		_, err = btcwire.TstReadVarBytes(r, test.pver,
-			"alert serialized payload")
+			btcwire.MaxMessagePayload, "test payload")
 		if err != test.readErr {
 			t.Errorf("readVarBytes #%d wrong error got: %v, want: %v",
 				i, err, test.readErr)
@@ -623,8 +623,8 @@ func TestVarBytesOverflowErrors(t *testing.T) {
 	for i, test := range tests {
 		// Decode from wire format.
 		rbuf := bytes.NewBuffer(test.buf)
-		_, err := btcwire.TstReadVarBytes(rbuf, test.pver, btcwire.MaxMessagePayload,
+		_, err := btcwire.TstReadVarBytes(rbuf, test.pver,
-			"alert serialized payload")
+			btcwire.MaxMessagePayload, "test payload")
 		if reflect.TypeOf(err) != reflect.TypeOf(test.err) {
 			t.Errorf("readVarBytes #%d wrong error got: %v, "+
 				"want: %v", i, err, reflect.TypeOf(test.err))

msgalert.go

@@ -58,46 +58,49 @@ import (
 //        |   Total  (Fixed)                |   45      |
 //        -----------------------------------------------
 //
-// note:
+// NOTE:
 //      * string is a VarString i.e VarInt length followed by the string itself
 //      * set<string> is a VarInt followed by as many number of strings
 //      * set<int32> is a VarInt followed by as many number of ints
 //      * fixedAlertSize = 40 + 5*min(VarInt)  = 40 + 5*1 = 45
-
+//
 // Now we can define bounds on Alert size, SetCancel and SetSubVer
 
 // Fixed size of the alert payload
 const fixedAlertSize = 45
 
-// Max size of the ECDSA signature
+// maxSignatureSize is the max size of an ECDSA signature.
-// note: since this size is fixed and < 255, size of VarInt
+// NOTE: Since this size is fixed and < 255, the size of VarInt required = 1.
-// required = 1 (fits in uint8)
 const maxSignatureSize = 72
 
-// Maximum size of the alert
+// maxAlertSize is the maximum size an alert.
+//
 // MessagePayload = VarInt(Alert) + Alert + VarInt(Signature) + Signature
 // maxMessagePayload = maxAlertSize + max(VarInt) + maxSignatureSize + 1
 const maxAlertSize = maxMessagePayload - maxSignatureSize - MaxVarIntPayload - 1
 
-// Maximum number of Cancel IDs from SetCancel to read
+// maxCountSetCancel is the maximum number of cancel IDs that could possibly
+// fit into a maximum size alert.
+//
 // maxAlertSize = fixedAlertSize + max(SetCancel) + max(SetSubVer) + 3*(string)
-// for caculating maximum number of Cancel IDs, set all other variable sizes to 0
+// for caculating maximum number of cancel IDs, set all other var  sizes to 0
 // maxAlertSize = fixedAlertSize + (MaxVarIntPayload-1) + x*sizeOf(int32)
 // x = (maxAlertSize - fixedAlertSize - MaxVarIntPayload + 1) / 4
 const maxCountSetCancel = (maxAlertSize - fixedAlertSize - MaxVarIntPayload + 1) / 4
 
-// Maximum number of subversions from SetSubVer to read
+// maxCountSetSubVer is the maximum number of subversions that could possibly
+// fit into a maximum size alert.
+//
 // maxAlertSize = fixedAlertSize + max(SetCancel) + max(SetSubVer) + 3*(string)
-// for caculating maximum number of subversions, set all other variable sizes to 0
+// for caculating maximum number of subversions, set all other var sizes to 0
 // maxAlertSize = fixedAlertSize + (MaxVarIntPayload-1) + x*sizeOf(string)
 // x = (maxAlertSize - fixedAlertSize - MaxVarIntPayload + 1) / sizeOf(string)
 // subversion would typically be something like "/Satoshi:0.7.2/" (15 bytes)
 // so assuming < 255 bytes, sizeOf(string) = sizeOf(uint8) + 255 = 256
 const maxCountSetSubVer = (maxAlertSize - fixedAlertSize - MaxVarIntPayload + 1) / 256
 
-// Alert contains the data deserialized from the MsgAlert payload
+// Alert contains the data deserialized from the MsgAlert payload.
 type Alert struct {
-
 	// Alert format version
 	Version int32
 
@@ -144,41 +147,7 @@ type Alert struct {
 	Reserved string
 }
 
-// NewAlert returns an new Alert with values provided
+// Serialize encodes the alert to w using the alert protocol encoding format.
-func NewAlert(version int32, relayuntil int64, expiration int64,
-	id int32, cancel int32, setcancel []int32, minver int32,
-	maxver int32, setsubver []string, priority int32, comment string,
-	statusbar string, reserved string) *Alert {
-	return &Alert{
-		Version:    version,
-		RelayUntil: relayuntil,
-		Expiration: expiration,
-		ID:         id,
-		Cancel:     cancel,
-		SetCancel:  setcancel,
-		MinVer:     minver,
-		MaxVer:     maxver,
-		SetSubVer:  setsubver,
-		Priority:   priority,
-		Comment:    comment,
-		StatusBar:  statusbar,
-		Reserved:   reserved,
-	}
-}
-
-// NewAlertFromPayload returns an Alert with values deserialized
-// from the serializedpayload
-func NewAlertFromPayload(serializedpayload []byte, pver uint32) (*Alert, error) {
-	var alert Alert
-	r := bytes.NewReader(serializedpayload)
-	err := alert.Deserialize(r, pver)
-	if err != nil {
-		return nil, err
-	}
-	return &alert, nil
-}
-
-// Serialize writes a serialized byte array of the Alert
 func (alert *Alert) Serialize(w io.Writer, pver uint32) error {
 	err := writeElements(w, &alert.Version,
 		&alert.RelayUntil, &alert.Expiration, &alert.ID, &alert.Cancel)
@@ -244,8 +213,8 @@ func (alert *Alert) Serialize(w io.Writer, pver uint32) error {
 	return nil
 }
 
-// Deserialize reads a byte array, deserializes
+// Deserialize decodes from r into the receiver using the alert protocol
-// it and updates the Alert
+// encoding format.
 func (alert *Alert) Deserialize(r io.Reader, pver uint32) error {
 	err := readElements(r, &alert.Version, &alert.RelayUntil,
 		&alert.Expiration, &alert.ID, &alert.Cancel)
@@ -316,6 +285,40 @@ func (alert *Alert) Deserialize(r io.Reader, pver uint32) error {
 	return nil
 }
 
+// NewAlert returns an new Alert with values provided.
+func NewAlert(version int32, relayUntil int64, expiration int64,
+	id int32, cancel int32, setCancel []int32, minVer int32,
+	maxVer int32, setSubVer []string, priority int32, comment string,
+	statusBar string) *Alert {
+	return &Alert{
+		Version:    version,
+		RelayUntil: relayUntil,
+		Expiration: expiration,
+		ID:         id,
+		Cancel:     cancel,
+		SetCancel:  setCancel,
+		MinVer:     minVer,
+		MaxVer:     maxVer,
+		SetSubVer:  setSubVer,
+		Priority:   priority,
+		Comment:    comment,
+		StatusBar:  statusBar,
+		Reserved:   "",
+	}
+}
+
+// NewAlertFromPayload returns an Alert with values deserialized from the
+// serialized payload.
+func NewAlertFromPayload(serializedPayload []byte, pver uint32) (*Alert, error) {
+	var alert Alert
+	r := bytes.NewReader(serializedPayload)
+	err := alert.Deserialize(r, pver)
+	if err != nil {
+		return nil, err
+	}
+	return &alert, nil
+}
+
 // MsgAlert  implements the Message interface and defines a bitcoin alert
 // message.
 //
@@ -410,9 +413,9 @@ func (msg *MsgAlert) MaxPayloadLength(pver uint32) uint32 {
 
 // NewMsgAlert returns a new bitcoin alert message that conforms to the Message
 // interface.  See MsgAlert for details.
-func NewMsgAlert(serializedpayload []byte, signature []byte) *MsgAlert {
+func NewMsgAlert(serializedPayload []byte, signature []byte) *MsgAlert {
 	return &MsgAlert{
-		SerializedPayload: serializedpayload,
+		SerializedPayload: serializedPayload,
 		Signature:         signature,
 		Payload:           nil,
 	}

msgalert_test.go

@@ -283,7 +283,7 @@ func TestAlert(t *testing.T) {
 	alert := btcwire.NewAlert(
 		1, 1337093712, 1368628812, 1015,
 		1013, []int32{1014}, 0, 40599, []string{"/Satoshi:0.7.2/"}, 5000, "",
-		"URGENT: upgrade required, see http://bitcoin.org/dos for details", "",
+		"URGENT: upgrade required, see http://bitcoin.org/dos for details",
 	)
 	w := new(bytes.Buffer)
 	err := alert.Serialize(w, pver)
@@ -370,7 +370,7 @@ func TestAlertErrors(t *testing.T) {
 	baseAlert := btcwire.NewAlert(
 		1, 1337093712, 1368628812, 1015,
 		1013, []int32{1014}, 0, 40599, []string{"/Satoshi:0.7.2/"}, 5000, "",
-		"URGENT", "",
+		"URGENT",
 	)
 	baseAlertEncoded := []byte{
 		0x01, 0x00, 0x00, 0x00, 0x50, 0x6e, 0xb2, 0x4f, 0x00, 0x00, 0x00, 0x00, 0x4c, 0x9e, 0x93, 0x51, //|....Pn.O....L..Q|