fix: Bugfixes and cleanup

sdk/listener.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"sync"
 
 	abcitypes "github.com/cometbft/cometbft/abci/types"
 	cometclient "github.com/cometbft/cometbft/rpc/client"
@@ -25,14 +26,14 @@ type TxListener struct {
 	cleanupFn func()
 }
 
-// NewTxListener creates a new listenver from a comet client
+// NewTxListener creates a new listener from a comet client
 func NewTxListener(client cometclient.Client) TxListener {
 	return TxListener{
 		rpc: client,
 	}
 }
 
-// Event models a Cometbft Tx event with unmarsheled Msg responses
+// Event models a Cometbft Tx event with unmarshaled Msg responses
 type Event struct {
 	Height    int64     `json:"height"`
 	Index     uint32    `json:"index"`
@@ -110,11 +111,11 @@ func (l *TxListener) ListenTxs(ctx context.Context) (<-chan Event, <-chan error,
 	return resultCh, errChn, err
 }
 
-// ListenAsync spawns a go routine and listens for txs asyncrhonously,
+// ListenAsync spawns a go routine and listens for txs asynchronously,
 // until the comet client closes the connection, the context is cancelled.
 // or the listener is closed.
 // Callback is called each time an event or an error is received
-// Returns an error if connection to commet fails
+// Returns an error if connection to comet fails
 func (l *TxListener) ListenAsync(ctx context.Context, cb func(*Event, error)) error {
 	evs, errs, err := l.ListenTxs(ctx)
 	if err != nil {
@@ -130,9 +131,9 @@ func (l *TxListener) ListenAsync(ctx context.Context, cb func(*Event, error)) er
 				cb(nil, err)
 			case <-l.Done():
 				log.Printf("Listener closed: canceling loop")
-				break
+				return
 			case <-ctx.Done():
-				break
+				return
 			}
 		}
 	}()
@@ -155,26 +156,22 @@ func (l *TxListener) Close() {
 func channelMapper[T, U any](ch <-chan T, mapper func(T) (U, error)) (values <-chan U, errors <-chan error, closeFn func()) {
 	errCh := make(chan error, mapperBuffSize)
 	valCh := make(chan U, mapperBuffSize)
-	closeFn = func() {
+	var once sync.Once
+	doClose := func() {
 		close(errCh)
 		close(valCh)
 	}
+	closeFn = func() {
+		once.Do(doClose)
+	}
 	go func() {
-		for {
-			select {
-			case result, ok := <-ch:
-				if !ok {
-					close(errCh)
-					close(valCh)
-					return
-				}
-
-				u, err := mapper(result)
-				if err != nil {
-					errCh <- err
-				} else {
-					valCh <- u
-				}
+		defer once.Do(doClose)
+		for result := range ch {
+			u, err := mapper(result)
+			if err != nil {
+				errCh <- err
+			} else {
+				valCh <- u
 			}
 		}
 	}()

utils/sort.go

@@ -69,7 +69,7 @@ func (s Sortable[T]) SortInPlace() {
 
 // Sort returns a sorted slice of the elements given originally
 func (s Sortable[T]) Sort() []T {
-	vals := make([]T, 0, len(s.ts))
+	vals := make([]T, len(s.ts))
 	copy(vals, s.ts)
 	sortable := Sortable[T]{
 		ts:         vals,
@@ -83,7 +83,6 @@ func (s Sortable[T]) Sort() []T {
 func SortSlice[T Ordered](elems []T) {
 	sortable := Sortable[T]{
 		ts: elems,
-		//comparator: comparator,
 		comparator: func(left T, right T) bool { return left < right },
 	}
 	sortable.SortInPlace()

utils/span.go

@@ -66,11 +66,19 @@ func WithMsgSpan(ctx sdk.Context) sdk.Context {
 }
 
 func GetMsgSpan(ctx sdk.Context) *MsgSpan {
-	return ctx.Context().Value(spanCtxKey).(*MsgSpan)
+	span, ok := ctx.Context().Value(spanCtxKey).(*MsgSpan)
+	if !ok {
+		return nil
+	}
+	return span
 }
 
 // FinalizeSpan ends the span duration frame, transforms it into an SDK Event and emits it using the event manager
 func FinalizeSpan(ctx sdk.Context) {
-	event := GetMsgSpan(ctx).ToEvent()
+	span := GetMsgSpan(ctx)
+	if span == nil {
+		return
+	}
+	event := span.ToEvent()
 	ctx.EventManager().EmitEvent(event)
 }

x/acp/capability/manager.go

@@ -130,12 +130,10 @@ func (m *PolicyCapabilityManager) isOwnedByAcpModule(ctx sdk.Context, capability
 		return false, fmt.Errorf("looking up capability owner: %v", err)
 	}
 
-	mods = utils.FilterSlice(mods, func(name string) bool {
-		return name != types.ModuleName
-	})
-
-	if len(mods) == 0 {
-		return false, nil
+	for _, mod := range mods {
+		if mod == types.ModuleName {
+			return true, nil
+		}
 	}
-	return true, nil
+	return false, nil
 }

x/acp/capability/manager_test.go

@@ -0,0 +1,145 @@
+package capability
+
+import (
+	"testing"
+
+	"cosmossdk.io/log"
+	"cosmossdk.io/store"
+	"cosmossdk.io/store/metrics"
+	storetypes "cosmossdk.io/store/types"
+	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	dbm "github.com/cosmos/cosmos-db"
+	"github.com/cosmos/cosmos-sdk/codec"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper"
+	"github.com/stretchr/testify/require"
+)
+
+func setupCapKeeper(t *testing.T) (sdk.Context, *capabilitykeeper.ScopedKeeper, *capabilitykeeper.ScopedKeeper) {
+	capStoreKey := storetypes.NewKVStoreKey("capkeeper")
+	capMemStoreKey := storetypes.NewMemoryStoreKey("capkeepermem")
+
+	db := dbm.NewMemDB()
+	stateStore := store.NewCommitMultiStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
+	stateStore.MountStoreWithDB(capStoreKey, storetypes.StoreTypeDB, db)
+	stateStore.MountStoreWithDB(capMemStoreKey, storetypes.StoreTypeMemory, nil)
+	require.NoError(t, stateStore.LoadLatestVersion())
+
+	registry := codectypes.NewInterfaceRegistry()
+	cdc := codec.NewProtoCodec(registry)
+	capKeeper := capabilitykeeper.NewKeeper(cdc, capStoreKey, capMemStoreKey)
+
+	acpScoped := capKeeper.ScopeToModule("acp")
+	otherScoped := capKeeper.ScopeToModule("other")
+
+	ctx := sdk.NewContext(stateStore, cmtproto.Header{}, false, log.NewNopLogger())
+	capKeeper.Seal()
+
+	return ctx, &acpScoped, &otherScoped
+}
+
+func TestNewPolicyCapabilityManager(t *testing.T) {
+	_, acpScoped, _ := setupCapKeeper(t)
+	mgr := NewPolicyCapabilityManager(acpScoped)
+	require.NotNil(t, mgr)
+}
+
+func TestIssueAndFetch(t *testing.T) {
+	ctx, acpScoped, _ := setupCapKeeper(t)
+	mgr := NewPolicyCapabilityManager(acpScoped)
+
+	cap, err := mgr.Issue(ctx, "policy-1")
+	require.NoError(t, err)
+	require.NotNil(t, cap)
+	require.Equal(t, "policy-1", cap.GetPolicyId())
+	require.NotNil(t, cap.GetCosmosCapability())
+
+	fetched, err := mgr.Fetch(ctx, "policy-1")
+	require.NoError(t, err)
+	require.NotNil(t, fetched)
+	require.Equal(t, "policy-1", fetched.GetPolicyId())
+}
+
+func TestFetchNonExistent(t *testing.T) {
+	ctx, acpScoped, _ := setupCapKeeper(t)
+	mgr := NewPolicyCapabilityManager(acpScoped)
+
+	_, err := mgr.Fetch(ctx, "nonexistent")
+	require.Error(t, err)
+}
+
+func TestClaimCapability(t *testing.T) {
+	ctx, acpScoped, otherScoped := setupCapKeeper(t)
+	acpMgr := NewPolicyCapabilityManager(acpScoped)
+	otherMgr := NewPolicyCapabilityManager(otherScoped)
+
+	// acp issues
+	cap, err := acpMgr.Issue(ctx, "policy-1")
+	require.NoError(t, err)
+
+	// other module claims
+	err = otherMgr.Claim(ctx, cap)
+	require.NoError(t, err)
+
+	// other module can now fetch
+	fetched, err := otherMgr.Fetch(ctx, "policy-1")
+	require.NoError(t, err)
+	require.NotNil(t, fetched)
+}
+
+func TestValidateCapability(t *testing.T) {
+	ctx, acpScoped, otherScoped := setupCapKeeper(t)
+	acpMgr := NewPolicyCapabilityManager(acpScoped)
+	otherMgr := NewPolicyCapabilityManager(otherScoped)
+
+	cap, err := acpMgr.Issue(ctx, "policy-1")
+	require.NoError(t, err)
+
+	err = otherMgr.Claim(ctx, cap)
+	require.NoError(t, err)
+
+	// validate from the other module's perspective — acp is an owner, so it passes
+	err = otherMgr.Validate(ctx, cap)
+	require.NoError(t, err)
+}
+
+func TestGetOwnerModule(t *testing.T) {
+	ctx, acpScoped, otherScoped := setupCapKeeper(t)
+	acpMgr := NewPolicyCapabilityManager(acpScoped)
+	otherMgr := NewPolicyCapabilityManager(otherScoped)
+
+	cap, err := acpMgr.Issue(ctx, "policy-1")
+	require.NoError(t, err)
+
+	err = otherMgr.Claim(ctx, cap)
+	require.NoError(t, err)
+
+	owner, err := otherMgr.GetOwnerModule(ctx, cap)
+	require.NoError(t, err)
+	require.Equal(t, "other", owner)
+}
+
+func TestGetOwnerModuleNoClaimer(t *testing.T) {
+	ctx, acpScoped, _ := setupCapKeeper(t)
+	acpMgr := NewPolicyCapabilityManager(acpScoped)
+
+	cap, err := acpMgr.Issue(ctx, "policy-1")
+	require.NoError(t, err)
+
+	// only acp owns it, after filtering acp out, mods is empty
+	_, err = acpMgr.GetOwnerModule(ctx, cap)
+	require.Error(t, err)
+}
+
+func TestValidateAcpOnlyOwner(t *testing.T) {
+	ctx, acpScoped, _ := setupCapKeeper(t)
+	acpMgr := NewPolicyCapabilityManager(acpScoped)
+
+	cap, err := acpMgr.Issue(ctx, "policy-1")
+	require.NoError(t, err)
+
+	// Validate should pass since acp issued it
+	err = acpMgr.Validate(ctx, cap)
+	require.NoError(t, err)
+}

x/acp/capability/types_test.go

@@ -0,0 +1,22 @@
+package capability
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestPolicyCapabilityGetCapabilityName(t *testing.T) {
+	cap := &PolicyCapability{policyId: "test-policy-123"}
+	require.Equal(t, "/acp/module_policies/test-policy-123", cap.GetCapabilityName())
+}
+
+func TestPolicyCapabilityGetPolicyId(t *testing.T) {
+	cap := &PolicyCapability{policyId: "test-policy-123"}
+	require.Equal(t, "test-policy-123", cap.GetPolicyId())
+}
+
+func TestPolicyCapabilityGetCosmosCapability(t *testing.T) {
+	cap := &PolicyCapability{policyId: "p1", capability: nil}
+	require.Nil(t, cap.GetCosmosCapability())
+}

x/acp/did/did_test.go

@@ -0,0 +1,24 @@
+package did
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestProduceDID(t *testing.T) {
+	did, signer, err := ProduceDID()
+	require.NoError(t, err)
+	require.NotEmpty(t, did)
+	require.NotNil(t, signer)
+	require.Contains(t, did, "did:key:")
+	require.NoError(t, IsValidDID(did))
+}
+
+func TestProduceDIDUniqueness(t *testing.T) {
+	did1, _, err := ProduceDID()
+	require.NoError(t, err)
+	did2, _, err := ProduceDID()
+	require.NoError(t, err)
+	require.NotEqual(t, did1, did2)
+}

x/acp/did/types.go

@@ -42,6 +42,9 @@ func IssueDID(acc sdk.AccountI) (string, error) {
 
 // DIDFromPubKey constructs and returns a DID from a public key.
 func DIDFromPubKey(pk cryptotypes.PubKey) (string, error) {
+	if pk == nil {
+		return "", fmt.Errorf("account public key is nil")
+	}
 	var keyType crypto.KeyType
 	switch t := pk.(type) {
 	case *secp256k1.PubKey: