Colin/comments (#28)

* updated comments

* Updated comments

* fixed merge

app/app.go

@@ -22,7 +22,7 @@ const (
 
 // Extended ABCI application
 type ChildChain struct {
-	*bam.BaseApp // Pointer to the Base App
+	*bam.BaseApp
 
 	cdc *amino.Codec
 
@@ -31,9 +31,9 @@ type ChildChain struct {
 	feeAmount *uint64
 
 	// keys to access the substores
-	capKeyMainStore *sdk.KVStoreKey //capabilities key to access main store from multistore
+	capKeyMainStore *sdk.KVStoreKey
 
-	// Manage addition and deletion of unspent utxo's
+	// Manage addition and deletion of utxo's
 	utxoMapper types.UTXOMapper
 
 	txHash []byte
@@ -61,7 +61,6 @@ func NewChildChain(logger log.Logger, db dbm.DB) *ChildChain {
 	app.Router().
 		AddRoute("txs", auth.NewHandler(UTXOKeeper, app.txIndex))
 
-	// initialize BaseApp
 	// set the BaseApp txDecoder to use txDecoder with RLP
 	app.SetTxDecoder(app.txDecoder)
 

app/app_test.go

@@ -3,7 +3,7 @@ package app
 import (
 	"os"
 	"testing"
-	//"fmt" //for debugging
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	ethcrypto "github.com/ethereum/go-ethereum/crypto"
 	"github.com/stretchr/testify/assert"
@@ -24,7 +24,9 @@ func newChildChain() *ChildChain {
 	return NewChildChain(logger, db)
 }
 
-func TestDepositMsg(t *testing.T) {
+// Attempts to spend a non-existent utxo
+// without depositing first.
+func TestBadSpendMsg(t *testing.T) {
 	cc := newChildChain()
 
 	confirmSigs := [2]crypto.Signature{crypto.SignatureSecp256k1{}, crypto.SignatureSecp256k1{}}
@@ -52,6 +54,7 @@ func TestDepositMsg(t *testing.T) {
 		Fee:          1,
 	}
 
+	// Signs the hash of the transaction
 	hash := ethcrypto.Keccak256(msg.GetSignBytes())
 	sig, _ := ethcrypto.Sign(hash, privKeyA)
 	tx := types.NewBaseTx(msg, []sdk.StdSignature{{

auth/ante.go

@@ -10,12 +10,13 @@ import (
 )
 
 // NewAnteHandler returns an AnteHandler that checks signatures,
-// and deducts fees from the first signer.
+// confirm signatures, and increments the feeAmount
 func NewAnteHandler(utxoMapper types.UTXOMapper, txIndex *uint16, feeAmount *uint64) sdk.AnteHandler {
 	return func(
 		ctx sdk.Context, tx sdk.Tx,
 	) (_ sdk.Context, _ sdk.Result, abort bool) {
 
+		// Assert that there are signatures
 		sigs := tx.GetSignatures()
 		if len(sigs) == 0 {
 			return ctx,
@@ -45,7 +46,7 @@ func NewAnteHandler(utxoMapper types.UTXOMapper, txIndex *uint16, feeAmount *uin
 		}
 		signBytes := spendMsg.GetSignBytes()
 
-		//Check that inputs are valid and having valid signatures
+		// Verify the first input signature
 		position1 := types.Position{spendMsg.Blknum1, spendMsg.Txindex1, spendMsg.Oindex1, spendMsg.DepositNum1}
 		res := processSig(ctx, utxoMapper, position1, signerAddrs[0], sigs[0], signBytes)
 
@@ -55,12 +56,13 @@ func NewAnteHandler(utxoMapper types.UTXOMapper, txIndex *uint16, feeAmount *uin
 
 		posSignBytes := position1.GetSignBytes()
 
-		//Check that confirm signature is valid
+		// Verify that confirmation signature
 		res = processConfirmSig(ctx, utxoMapper, position1, spendMsg.ConfirmSigs1, posSignBytes)
 		if !res.IsOK() {
 			return ctx, res, true
 		}
-		//Verify validity of second input
+
+		// Verify the second input
 		if utils.ValidAddress(spendMsg.Owner2) {
 			position2 := types.Position{spendMsg.Blknum2, spendMsg.Txindex2, spendMsg.Oindex2, spendMsg.DepositNum2}
 			res = processSig(ctx, utxoMapper, position2, signerAddrs[1], sigs[1], signBytes)
@@ -76,9 +78,8 @@ func NewAnteHandler(utxoMapper types.UTXOMapper, txIndex *uint16, feeAmount *uin
 			}
 		}
 
-		// If DeliverTx() update fee
+		// Increment amount of fees collected
 		if !ctx.IsCheckTx() {
-			//Increment amount of fees collected
 			(*feeAmount) += spendMsg.Fee
 		}
 
@@ -91,17 +92,22 @@ func processSig(
 	ctx sdk.Context, um types.UTXOMapper,
 	position types.Position, addr crypto.Address, sig sdk.StdSignature, signBytes []byte) (
 	res sdk.Result) {
-	// Check UTXO is not nil.
+
+	// Verify utxo exists
 	utxo := um.GetUTXO(ctx, position)
 	if utxo == nil {
 		return sdk.ErrUnknownRequest("UTXO trying to be spent, does not exist").Result()
 	}
 
-	//Checks that utxo owner equals address in the spendmsg
+	// Verify that utxo owner equals input address in the transaction
 	if !reflect.DeepEqual(utxo.GetAddress().Bytes(), addr.Bytes()) {
 		return sdk.ErrUnauthorized("signer does not match utxo owner").Result()
 	}
 
+	// sig.Signature.Bytes() returns amino encoded signature
+	// the first 5 bytes represent the encoding for the signature
+	// Bytes 1-4: prefix bytes (crypto.SignatureSecp256k1 has prefix of 0x16E1FEEA)
+	// Byte 5: byte array length (should be 0x41 or 65)
 	hash := ethcrypto.Keccak256(signBytes)
 	pubKey1, err1 := ethcrypto.SigToPub(hash, sig.Signature.Bytes()[5:])
 
@@ -117,6 +123,7 @@ func processConfirmSig(
 	position types.Position, sig [2]crypto.Signature, signBytes []byte) (
 	res sdk.Result) {
 
+	// Verify utxo exists
 	utxo := utxoMapper.GetUTXO(ctx, position)
 	if utxo == nil {
 		return sdk.ErrUnknownRequest("UTXO trying to be spent, does not exist").Result()
@@ -130,6 +137,10 @@ func processConfirmSig(
 
 	hash := ethcrypto.Keccak256(signBytes)
 
+	// sig.Signature.Bytes() returns amino encoded signature
+	// the first 5 bytes represent the encoding for the signature
+	// Bytes 1-4: prefix bytes (crypto.SignatureSecp256k1 has prefix of 0x16E1FEEA)
+	// Byte 5: byte array length (should be 0x41 or 65)
 	pubKey1, err1 := ethcrypto.SigToPub(hash, ethsigs[0].Bytes()[5:])
 	if err1 != nil || !reflect.DeepEqual(ethcrypto.PubkeyToAddress(*pubKey1).Bytes(), inputAddresses[0].Bytes()) {
 		return sdk.ErrUnauthorized("confirm signature 1 verification failed").Result()

auth/ante_test.go

@@ -95,6 +95,7 @@ func GenSpendMsgWithAddresses() types.SpendMsg {
 	}
 }
 
+// No signatures are provided
 func TestNoSigs(t *testing.T) {
 	ctx, mapper, txIndex, feeAmount := setup()
 
@@ -108,6 +109,7 @@ func TestNoSigs(t *testing.T) {
 	assert.Equal(t, sdk.ToABCICode(sdk.CodespaceType(1), sdk.CodeType(4)), res.Code, "Tx had processed with no signatures")
 }
 
+// The wrong amount of signatures are provided
 func TestNotEnoughSigs(t *testing.T) {
 	ctx, mapper, txIndex, feeAmount := setup()
 
@@ -127,9 +129,11 @@ func TestNotEnoughSigs(t *testing.T) {
 	assert.Equal(t, sdk.ToABCICode(sdk.CodespaceType(1), sdk.CodeType(4)), res.Code, "Tx had processed with incorrect number of signatures")
 }
 
+// The transaction is not signed by the utxo owner
 func TestWrongSigner(t *testing.T) {
 	ctx, mapper, txIndex, feeAmount := setup()
 
+	// Generate input utxos
 	position1 := types.Position{1000, 0, 0, 0}
 	position2 := types.Position{1000, 1, 0, 0}
 	privA, _ := ethcrypto.GenerateKey()
@@ -138,6 +142,8 @@ func TestWrongSigner(t *testing.T) {
 	utxo2 := NewUTXO(privA, privB, position2)
 	mapper.AddUTXO(ctx, utxo1)
 	mapper.AddUTXO(ctx, utxo2)
+
+	// Signature by non owner
 	var msg = GenSpendMsgWithAddresses()
 	msg.Owner1 = utils.EthPrivKeyToSDKAddress(privB)
 	msg.Owner2 = utils.EthPrivKeyToSDKAddress(privB)
@@ -159,7 +165,7 @@ func TestWrongSigner(t *testing.T) {
 	assert.Equal(t, sdk.ToABCICode(sdk.CodespaceType(1), sdk.CodeType(4)), res.Code, "Signer address does not match owner address")
 }
 
-//Tests a valid single input transaction
+// Tests a valid single input transaction
 func TestValidSingleInput(t *testing.T) {
 	ctx, mapper, txIndex, feeAmount := setup()
 
@@ -192,6 +198,8 @@ func TestValidSingleInput(t *testing.T) {
 		Denom2:       45,
 		Fee:          5,
 	}
+
+	// Sign transaction
 	hash := ethcrypto.Keccak256(msg.GetSignBytes())
 	sig, _ := ethcrypto.Sign(hash, privKeyA)
 	sig1 := crypto.SignatureSecp256k1(sig)
@@ -215,13 +223,14 @@ func TestValidSingleInput(t *testing.T) {
 	assert.Equal(t, sdk.ToABCICode(sdk.CodespaceType(1), sdk.CodeType(0)), res.Code, res.Log)
 }
 
-//Tests a valid transaction
+// Tests a valid transaction
 func TestValidTransaction(t *testing.T) {
 	ctx, mapper, txIndex, feeAmount := setup()
 
 	privKeyA, _ := ethcrypto.GenerateKey() //Input Owner
 	privKeyB, _ := ethcrypto.GenerateKey() //ConfirmSig owner and recipient
 
+	// Generate valid inputs
 	position1 := types.Position{1, 0, 0, 0}
 	position2 := types.Position{1, 1, 0, 0}
 	confirmSigHash1 := ethcrypto.Keccak256(position1.GetSignBytes())

auth/handler.go

@@ -21,7 +21,9 @@ func NewHandler(uk db.UTXOKeeper, txIndex *uint16) sdk.Handler {
 }
 
 // Handle SpendMsg.
+// Spends inputs, creates new outputs
 func handleSpendMsg(ctx sdk.Context, uk db.UTXOKeeper, msg types.SpendMsg, txIndex *uint16) sdk.Result {
+
 	position1 := types.Position{msg.Blknum1, msg.Txindex1, msg.Oindex1, msg.DepositNum1}
 	utxo1 := uk.UM.GetUTXO(ctx, position1)
 	var position2 types.Position

auth/handler_test.go

@@ -25,11 +25,8 @@ func NewUTXO(privA *ecdsa.PrivateKey, privB *ecdsa.PrivateKey, position types.Po
 	return types.NewBaseUTXO(addrB, confirmAddr, 100, position)
 }
 
-/*
-	Tests a valid spendmsg
-	2 different inputs and 2 different outputs
-	Inputs are from the same block
-*/
+// Tests a valid spendmsg
+// 2 different inputs and 2 different outputs
 func TestHandleSpendMessage(t *testing.T) {
 	ms, capKey := db.SetupMultiStore()
 
@@ -109,10 +106,8 @@ func TestHandleSpendMessage(t *testing.T) {
 	assert.EqualValues(t, inputAddresses, utxo2.GetInputAddresses())
 }
 
-/*
-	Tests a valid spendmsg
-	1 input and 2 different outputs
-*/
+// Tests a valid spendmsg
+// 1 input and 2 different outputs
 func TestOneInput(t *testing.T) {
 	ms, capKey := db.SetupMultiStore()
 

db/keeper.go

@@ -19,7 +19,7 @@ func NewUTXOKeeper(um types.UTXOMapper) UTXOKeeper {
 func (uk UTXOKeeper) SpendUTXO(ctx sdk.Context, addr crypto.Address, position types.Position) sdk.Error {
 
 	utxo := uk.UM.GetUTXO(ctx, position) // Get the utxo that should be spent
-	// Check to see if utxo exists, will be taken care of in ante handler
+
 	if utxo == nil {
 		return types.ErrInvalidUTXO(types.DefaultCodespace, "Unrecognized UTXO. Does not exist.")
 	}
@@ -30,12 +30,11 @@ func (uk UTXOKeeper) SpendUTXO(ctx sdk.Context, addr crypto.Address, position ty
 // Creates a new utxo and adds it to the utxo store
 func (uk UTXOKeeper) RecieveUTXO(ctx sdk.Context, addr crypto.Address, denom uint64,
 	oldutxos [2]types.UTXO, oindex uint8, txIndex uint16) sdk.Error {
-	var inputAddr1 crypto.Address
+
+	inputAddr1 := oldutxos[0].GetAddress()
 	var inputAddr2 crypto.Address
-	if oldutxos[0] != nil {
-		inputAddr1 = oldutxos[0].GetAddress()
-	}
 
+	// oldutxo[1] may be nil
 	if oldutxos[1] != nil {
 		inputAddr2 = oldutxos[1].GetAddress()
 	}

db/mapper_test.go

@@ -36,10 +36,6 @@ func TestUTXOGetAddDelete(t *testing.T) {
 	positionB := types.Position{1000, 0, 0, 0}
 	confirmAddr := [2]crypto.Address{addrA, addrA}
 
-	// These lines of code error. Why?
-	//utxo := mapper.GetUXTO(ctx, positionB)
-	//assert.Nil(t, utxo)
-
 	utxo := types.NewBaseUTXO(addrB, confirmAddr, 100, positionB)
 	assert.NotNil(t, utxo)
 	assert.Equal(t, addrB, utxo.GetAddress())

types/tx.go

@@ -97,7 +97,7 @@ func (msg SpendMsg) String() string {
 
 // Implements Msg.
 func (msg SpendMsg) Get(key interface{}) (value interface{}) {
-	return nil // TODO: Implement
+	return nil
 }
 
 // Implements Msg.

types/tx_test.go

@@ -71,13 +71,15 @@ func GenSpendMsgWithAddresses() SpendMsg {
 	}
 }
 
+// Creates a transaction with no owners
 func TestNoOwners(t *testing.T) {
 	var msg = GenBasicSpendMsg()
 	err := msg.ValidateBasic()
 	assert.Equal(t, sdk.CodeType(101),
 		err.Code(), err.Error())
 }
 
+// Creates a transaction with no recipients
 func TestNoRecipients(t *testing.T) {
 	privKeyA, _ := ethcrypto.GenerateKey()
 	var msg = GenBasicSpendMsg()
@@ -88,6 +90,7 @@ func TestNoRecipients(t *testing.T) {
 		err.Code(), err.Error())
 }
 
+// The oindex is neither 0 or 1
 func TestIncorrectOIndex(t *testing.T) {
 	var msg1 = GenSpendMsgWithAddresses()
 	msg1.Oindex1 = 2
@@ -104,6 +107,7 @@ func TestIncorrectOIndex(t *testing.T) {
 
 }
 
+// Creates an invalid transaction referencing utxo and deposit
 func TestInvalidSpendDeposit(t *testing.T) {
 	var msg1 = GenSpendMsgWithAddresses()
 	msg1.DepositNum1 = 5
@@ -112,6 +116,7 @@ func TestInvalidSpendDeposit(t *testing.T) {
 	assert.Equal(t, sdk.CodeType(106), err.Code(), err.Error())
 }
 
+// Tests GetSigners method
 func TestGetSigners(t *testing.T) {
 	msg := GenBasicSpendMsg()
 	privKeyA, _ := ethcrypto.GenerateKey()

types/utxo.go

@@ -9,7 +9,6 @@ import (
 )
 
 // UTXO is a standard unspent transaction output
-// Has pubkey for authentication
 type UTXO interface {
 	// Address to which UTXO's are sent
 	GetAddress() crypto.Address
@@ -18,7 +17,6 @@ type UTXO interface {
 	GetInputAddresses() [2]crypto.Address
 	SetInputAddresses([2]crypto.Address) error
 
-	//Get and Set denomination of the utxo. Is uint64 appropriate type?
 	GetDenom() uint64
 	SetDenom(uint64) error //errors if already set
 
@@ -27,14 +25,9 @@ type UTXO interface {
 
 	Get(key interface{}) (value interface{}, err error)
 	Set(key interface{}, value interface{}) error
-
-	//TODO: ADD SUPPORT FOR DIFFERENT COINS
-	//Will possibly have to add support for input positions
-	//Do not need to add Pubkey to struct. Instead client ecrecovers confirm signature to get Pubkey. Then call PubKey.Address() to verify and verify against InputAddresses.
-
 }
 
-// BaseUTXO must have all confirm signatures in order of most recent up until the signatures of the original depsosits.
+// Implements UTXO interface
 type BaseUTXO struct {
 	InputAddresses [2]crypto.Address
 	Address        crypto.Address
@@ -122,7 +115,6 @@ func (utxo BaseUTXO) SetPosition(blockNum uint64, txIndex uint16, oIndex uint8,
 
 //----------------------------------------
 // misc
-// total position = Position.Blknum * 1000000 + Position.TxIndex * 10 + Position.Oindex
 
 type Position struct {
 	Blknum     uint64