Clean up roles module with Rigels comments

modules/roles/handler.go

@@ -1,8 +1,6 @@
 package roles
 
 import (
-	"github.com/tendermint/tmlibs/log"
-
 	"github.com/tendermint/basecoin"
 	"github.com/tendermint/basecoin/errors"
 	"github.com/tendermint/basecoin/state"
@@ -11,7 +9,10 @@ import (
 //NameRole - name space of the roles module
 const NameRole = "role"
 
-type Handler struct{}
+// Handler allows us to create new roles
+type Handler struct {
+	basecoin.NopOption
+}
 
 var _ basecoin.Handler = Handler{}
 
@@ -25,13 +26,15 @@ func (Handler) Name() string {
 	return NameRole
 }
 
-// CheckTx checks if there is enough money in the account
+// 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
 }
 
-// DeliverTx moves the money
+// DeliverTx tries to create a new role.
+//
+// Returns an error if the role already exists
 func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx) (res basecoin.Result, err error) {
 	create, err := checkTx(ctx, tx)
 	if err != nil {
@@ -44,14 +47,6 @@ func (h Handler) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoi
 	return res, err
 }
 
-// SetOption - sets the genesis account balance
-func (h Handler) SetOption(l log.Logger, store state.KVStore, module, key, value string) (log string, err error) {
-	if module != NameRole {
-		return "", errors.ErrUnknownModule(module)
-	}
-	return "", errors.ErrUnknownKey(key)
-}
-
 func checkTx(ctx basecoin.Context, tx basecoin.Tx) (create CreateRoleTx, err error) {
 	// check if the tx is proper type and valid
 	create, ok := tx.Unwrap().(CreateRoleTx)

modules/roles/middleware.go

@@ -2,17 +2,19 @@ package roles
 
 import (
 	"github.com/tendermint/basecoin"
-	"github.com/tendermint/basecoin/errors"
 	"github.com/tendermint/basecoin/stack"
 	"github.com/tendermint/basecoin/state"
 )
 
+// Middleware allows us to add a requested role as a permission
+// if the tx requests it and has sufficient authority
 type Middleware struct {
 	stack.PassOption
 }
 
 var _ stack.Middleware = Middleware{}
 
+// NewMiddleware creates a role-checking middleware
 func NewMiddleware() Middleware {
 	return Middleware{}
 }
@@ -22,52 +24,50 @@ func (Middleware) Name() string {
 	return NameRole
 }
 
-// CheckTx checks if this is valid
+// CheckTx tries to assume the named role if requested.
+// If no role is requested, do nothing.
+// If insufficient authority to assume the role, return error.
 func (m Middleware) CheckTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Checker) (res basecoin.Result, err error) {
-	assume, err := checkMiddleTx(ctx, tx)
-	if err != nil {
-		return res, err
-	}
-
-	ctx, err = assumeRole(ctx, store, assume)
-	if err != nil {
-		return res, err
-	}
-
-	return next.CheckTx(ctx, store, assume.Tx)
-}
-
-// DeliverTx moves the money
-func (m Middleware) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) {
-	assume, err := checkMiddleTx(ctx, tx)
-	if err != nil {
-		return res, err
-	}
-
-	ctx, err = assumeRole(ctx, store, assume)
-	if err != nil {
-		return res, err
-	}
-
-	return next.DeliverTx(ctx, store, assume.Tx)
-}
-
-func checkMiddleTx(ctx basecoin.Context, tx basecoin.Tx) (assume AssumeRoleTx, err error) {
-	// check if the tx is proper type and valid
+	// if this is not an AssumeRoleTx, then continue
 	assume, ok := tx.Unwrap().(AssumeRoleTx)
-	if !ok {
-		return assume, errors.ErrInvalidFormat(tx)
-	}
-	err = assume.ValidateBasic()
-	if err != nil {
-		return assume, err
+	if !ok { // this also breaks the recursion below
+		return next.CheckTx(ctx, store, tx)
 	}
 
-	// load it up and check it out...
-	return assume, err
+	ctx, err = assumeRole(ctx, store, assume)
+	if err != nil {
+		return res, err
+	}
+
+	// one could add multiple role statements, repeat as needed
+	return m.CheckTx(ctx, store, assume.Tx, next)
+}
+
+// DeliverTx tries to assume the named role if requested.
+// If no role is requested, do nothing.
+// If insufficient authority to assume the role, return error.
+func (m Middleware) DeliverTx(ctx basecoin.Context, store state.KVStore, tx basecoin.Tx, next basecoin.Deliver) (res basecoin.Result, err error) {
+	// if this is not an AssumeRoleTx, then continue
+	assume, ok := tx.Unwrap().(AssumeRoleTx)
+	if !ok { // this also breaks the recursion below
+		return next.DeliverTx(ctx, store, tx)
+	}
+
+	ctx, err = assumeRole(ctx, store, assume)
+	if err != nil {
+		return res, err
+	}
+
+	// one could add multiple role statements, repeat as needed
+	return m.DeliverTx(ctx, store, assume.Tx, next)
 }
 
 func assumeRole(ctx basecoin.Context, store state.KVStore, assume AssumeRoleTx) (basecoin.Context, error) {
+	err := assume.ValidateBasic()
+	if err != nil {
+		return nil, err
+	}
+
 	role, err := loadRole(store, MakeKey(assume.Role))
 	if err != nil {
 		return nil, err

modules/roles/store.go

@@ -9,6 +9,7 @@ import (
 	wire "github.com/tendermint/go-wire"
 )
 
+// NewPerm creates a role permission with the given label
 func NewPerm(role []byte) basecoin.Actor {
 	return basecoin.Actor{
 		App:     NameRole,
@@ -22,6 +23,7 @@ type Role struct {
 	Signers []basecoin.Actor `json:"signers"`
 }
 
+// NewRole creates a Role structure to store the permissioning
 func NewRole(min uint32, signers []basecoin.Actor) Role {
 	return Role{
 		MinSigs: min,

modules/roles/tx.go

@@ -8,7 +8,10 @@ import (
 )
 
 const (
-	MaxMembers = 10
+	// MaxMembers it the maximum number of members in a Role.  Used to avoid
+	// extremely large roles.
+	// Value is arbitrary, please adjust as needed
+	MaxMembers = 20
 )
 
 // AssumeRoleTx is a layered tx that can wrap your normal tx to give it