From 7e7f124bc92fc0e580b98c63b8bef1ba64b0fd53 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Wed, 12 Jul 2017 16:22:17 +0200
Subject: [PATCH] Test create-role and fix CheckTx

---
 modules/roles/handler.go      |  9 +++++--
 modules/roles/handler_test.go | 50 +++++++++++++++++++++++++++++++++++
 modules/roles/store.go        | 11 ++++++--
 modules/roles/tx.go           |  2 +-
 4 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 modules/roles/handler_test.go

diff --git a/modules/roles/handler.go b/modules/roles/handler.go
index ce1ab137d..2ade96ee1 100644
--- a/modules/roles/handler.go
+++ b/modules/roles/handler.go
@@ -28,8 +28,13 @@ func (Handler) Name() string {
 
 // CheckTx verifies if the transaction is properly formated
 func (h Handler) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) {
-	_, err = checkTx(ctx, tx)
-	return res, err
+	var cr CreateRoleTx
+	cr, err = checkTx(ctx, tx)
+	if err != nil {
+		return
+	}
+	err = checkNoRole(store, cr.Role)
+	return
 }
 
 // DeliverTx tries to create a new role.
diff --git a/modules/roles/handler_test.go b/modules/roles/handler_test.go
new file mode 100644
index 000000000..e5c744bbc
--- /dev/null
+++ b/modules/roles/handler_test.go
@@ -0,0 +1,50 @@
+package roles_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/tendermint/basecoin"
+	"github.com/tendermint/basecoin/modules/roles"
+	"github.com/tendermint/basecoin/stack"
+	"github.com/tendermint/basecoin/state"
+)
+
+func TestCreateRole(t *testing.T) {
+	assert := assert.New(t)
+
+	a := basecoin.Actor{App: "foo", Address: []byte("bar")}
+	b := basecoin.Actor{ChainID: "eth", App: "foo", Address: []byte("bar")}
+	c := basecoin.Actor{App: "foo", Address: []byte("baz")}
+	d := basecoin.Actor{App: "si-ly", Address: []byte("bar")}
+
+	cases := []struct {
+		valid bool
+		role  string
+		min   uint32
+		sigs  []basecoin.Actor
+	}{
+		{true, "awesome", 1, []basecoin.Actor{a}},
+		{true, "cool", 2, []basecoin.Actor{b, c, d}},
+		{false, "oops", 3, []basecoin.Actor{a, d}}, // too many
+		{false, "ugh", 0, []basecoin.Actor{a, d}},  // too few
+		{false, "phew", 1, []basecoin.Actor{}},     // none
+		{false, "cool", 1, []basecoin.Actor{c, d}}, // duplicate of existing one
+	}
+
+	h := roles.NewHandler()
+	ctx := stack.MockContext("role-chain", 123)
+	store := state.NewMemKVStore()
+	for i, tc := range cases {
+		tx := roles.NewCreateRoleTx([]byte(tc.role), tc.min, tc.sigs)
+		_, err := h.CheckTx(ctx, store, tx)
+		_, err2 := h.DeliverTx(ctx, store, tx)
+		if tc.valid {
+			assert.Nil(err, "%d/%s: %+v", i, tc.role, err)
+			assert.Nil(err2, "%d/%s: %+v", i, tc.role, err2)
+		} else {
+			assert.NotNil(err, "%d/%s", i, tc.role)
+			assert.NotNil(err2, "%d/%s", i, tc.role)
+		}
+	}
+}
diff --git a/modules/roles/store.go b/modules/roles/store.go
index 4b3d4f3d1..c29d93f0f 100644
--- a/modules/roles/store.go
+++ b/modules/roles/store.go
@@ -68,11 +68,18 @@ func loadRole(store state.KVStore, key []byte) (role Role, err error) {
 	return role, nil
 }
 
-// we only have create here, no update, since we don't allow update yet
-func createRole(store state.KVStore, key []byte, role Role) error {
+func checkNoRole(store state.KVStore, key []byte) error {
 	if _, err := loadRole(store, key); !IsNoRoleErr(err) {
 		return ErrRoleExists()
 	}
+	return nil
+}
+
+// we only have create here, no update, since we don't allow update yet
+func createRole(store state.KVStore, key []byte, role Role) error {
+	if err := checkNoRole(store, key); err != nil {
+		return err
+	}
 	bin := wire.BinaryBytes(role)
 	store.Set(key, bin)
 	return nil // real stores can return error...
diff --git a/modules/roles/tx.go b/modules/roles/tx.go
index 6215f44e6..156b13a89 100644
--- a/modules/roles/tx.go
+++ b/modules/roles/tx.go
@@ -7,7 +7,7 @@ import (
 	"github.com/tendermint/basecoin/errors"
 )
 
-const (
+var (
 	// MaxMembers it the maximum number of members in a Role.  Used to avoid
 	// extremely large roles.
 	// Value is arbitrary, please adjust as needed