fix: bugfixes, comments

golang/cosmos/proto/agoric/lien/genesis.proto

@@ -14,10 +14,14 @@ message GenesisState {
     ];
 }
 
+// The lien on a particular account
 message AccountLien {
+    // Account address, bech32-encoded.
     string address = 1;
+
+    // The liened amount. Should be nonzero.
     repeated cosmos.base.v1beta1.Coin lien = 2 [
         (gogoproto.nullable) = false,
         (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"
     ]; 
-}
+}

golang/cosmos/proto/agoric/lien/lien.proto

@@ -0,0 +1,15 @@
+syntax = "proto3";
+package agoric.lien;
+
+import "gogoproto/gogo.proto";
+import "cosmos/base/v1beta1/coin.proto";
+
+option go_package = "github.com/Agoric/agoric-sdk/golang/cosmos/x/lien/types";
+
+// Lien contains the lien state of a particular account.
+message Lien {
+    repeated cosmos.base.v1beta1.Coin coins = 1 [
+        (gogoproto.nullable) = false,
+        (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"
+    ];
+}

golang/cosmos/x/lien/genesis.go

@@ -20,15 +20,26 @@ func ValidateGenesisState(genesisState types.GenesisState) error {
 }
 
 func InitGenesis(ctx sdk.Context, keeper Keeper, genesisState types.GenesisState) {
-	for _, lien := range genesisState.Liens {
-		keeper.SetAccountLien(ctx, lien)
+	for _, accLien := range genesisState.Liens {
+		addr, err := sdk.AccAddressFromBech32(accLien.GetAddress())
+		if err != nil {
+			panic(err) // not possible if genesis state was validated
+		}
+		lien := types.Lien{
+			Coins: accLien.GetLien(),
+		}
+		keeper.SetLien(ctx, addr, lien)
 	}
 }
 
 func ExportGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState {
 	genesisState := types.GenesisState{}
-	keeper.IterateAccountLiens(ctx, func(lien types.AccountLien) bool {
-		genesisState.Liens = append(genesisState.Liens, lien)
+	keeper.IterateLiens(ctx, func(addr sdk.AccAddress, lien types.Lien) bool {
+		accLien := types.AccountLien{
+			Address: addr.String(),
+			Lien:    lien.GetCoins(),
+		}
+		genesisState.Liens = append(genesisState.Liens, accLien)
 		return false
 	})
 	return genesisState

golang/cosmos/x/lien/keeper/account.go

@@ -11,9 +11,12 @@ import (
 	vestexported "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported"
 )
 
+// maxCoins returns coins with the maximum amount of each denomination
+// from its arguments.
 func maxCoins(a, b sdk.Coins) sdk.Coins {
 	max := make([]sdk.Coin, 0)
-	for indexA, indexB := 0, 0; indexA < len(a) && indexB < len(b); {
+	indexA, indexB := 0, 0
+	for indexA < len(a) && indexB < len(b) {
 		coinA, coinB := a[indexA], b[indexB]
 		switch strings.Compare(coinA.Denom, coinB.Denom) {
 		case -1: // A < B
@@ -34,113 +37,133 @@ func maxCoins(a, b sdk.Coins) sdk.Coins {
 			indexB++
 		}
 	}
+	// Any leftovers are by definition the maximum
+	for ; indexA < len(a); indexA++ {
+		max = append(max, a[indexA])
+	}
+	for ; indexB < len(b); indexB++ {
+		max = append(max, b[indexB])
+	}
+	// At most one of the previous two loops has run,
+	// so output should remain sorted.
 	return sdk.NewCoins(max...)
 }
 
-var _ vestexported.VestingAccount = &LienAccount{}
-
+// LienAccount wraps a VestingAccount to implement lien encumbrance.
+// The LockedCoins() method is the maximum of the coins locked for
+// liens, and the coins locked in the underlying VestingAccount.
+// It inherits the marshaling behavior of the wrapped account.
 type LienAccount struct {
 	vestexported.VestingAccount
 	lienKeeper Keeper
 }
 
+var _ vestexported.VestingAccount = LienAccount{}
+
 // LockedCoins implements the method from the VestingAccount interface.
 // It takes the maximum of the coins locked for liens and the cons
 // locked in the wrapped VestingAccount.
-func (la *LienAccount) LockedCoins(ctx sdk.Context) sdk.Coins {
+func (la LienAccount) LockedCoins(ctx sdk.Context) sdk.Coins {
 	wrappedLocked := la.VestingAccount.LockedCoins(ctx)
 	lienedLocked := la.LienedLockedCoins(ctx)
 	return maxCoins(wrappedLocked, lienedLocked)
 }
 
-func (la *LienAccount) LienedLockedCoins(ctx sdk.Context) sdk.Coins {
+// Returns the coins which are locked for lien encumbrance.
+func (la LienAccount) LienedLockedCoins(ctx sdk.Context) sdk.Coins {
 	state := la.lienKeeper.GetAccountState(ctx, la.GetAddress())
-	lien := la.lienKeeper.GetAccountLien(ctx, la.GetAddress())
-	return computeLienLocked(lien.GetLien(), state.Bonded, state.Unbonding)
+	return computeLienLocked(state.Liened, state.Bonded, state.Unbonding)
 }
 
+// Returns the coins which are locked for lien encumbrance.
+// The lien applies to bonded and unbonding coins before unbonded coins,
+// so we return max(0, liened - (bonded + unbonding)).
 func computeLienLocked(liened, bonded, unbonding sdk.Coins) sdk.Coins {
-	// We're only interested in the lien-encumbered unbonded coins.
-	// The bonded and unbonding coins are encumbered first, so we
-	// compute the remainder.
-	locked, _ := liened.SafeSub(bonded)
-	locked, _ = locked.SafeSub(unbonding)
-	return maxCoins(sdk.NewCoins(), locked)
+	// Since coins can't go negative, even transiently, we add then
+	// subtract the subtrahend:
+	//    max(0, A - B) = max(B, A) - B
+	subtrahend := bonded.Add(unbonding...)
+	return maxCoins(subtrahend, liened).Sub(subtrahend)
 }
 
+// NewAccountWrapper returns an AccountWrapper which wraps any account
+// to be a LienAccount associated with the given Keeper, then unwraps
+// any layers that the Wrap added.
 func NewAccountWrapper(lk Keeper) types.AccountWrapper {
 	return types.AccountWrapper{
 		Wrap: func(acc authtypes.AccountI) authtypes.AccountI {
 			if acc == nil {
 				return nil
 			}
-			return &LienAccount{
+			return LienAccount{
 				VestingAccount: makeVesting(acc),
 				lienKeeper:     lk,
 			}
 		},
 		Unwrap: func(acc authtypes.AccountI) authtypes.AccountI {
-			if la, ok := acc.(*LienAccount); ok {
+			if la, ok := acc.(LienAccount); ok {
 				acc = la.VestingAccount
 			}
-			if uva, ok := acc.(*unlockedVestingAccount); ok {
+			if uva, ok := acc.(unlockedVestingAccount); ok {
 				acc = uva.AccountI
 			}
 			return acc
 		},
 	}
 }
 
+// makeVesting returns a VestingAccount, wrapping a non-vesting argument
+// in a trivial implementation if necessary.
 func makeVesting(acc authtypes.AccountI) vestexported.VestingAccount {
 	if v, ok := acc.(vestexported.VestingAccount); ok {
 		return v
 	}
-	return &unlockedVestingAccount{AccountI: acc}
+	return unlockedVestingAccount{AccountI: acc}
 }
 
 // unlockedVestingAccount extends an ordinary account to be a vesting account
-// by simulating an original vesting amount of zero. It will be still stored
-// as the wrapped account.
+// by simulating an original vesting amount of zero. It inherets the marshal
+// behavior of the wrapped account.
 type unlockedVestingAccount struct {
 	authtypes.AccountI
 }
 
-var _ vestexported.VestingAccount = (*unlockedVestingAccount)(nil)
+var _ vestexported.VestingAccount = unlockedVestingAccount{}
 
-func (uva *unlockedVestingAccount) LockedCoins(ctx sdk.Context) sdk.Coins {
+func (uva unlockedVestingAccount) LockedCoins(ctx sdk.Context) sdk.Coins {
 	return sdk.NewCoins()
 }
 
-func (uva *unlockedVestingAccount) TrackDelegation(blockTime time.Time, balance, amount sdk.Coins) {
+func (uva unlockedVestingAccount) TrackDelegation(blockTime time.Time, balance, amount sdk.Coins) {
 }
 
-func (uva *unlockedVestingAccount) TrackUndelegation(amount sdk.Coins) {
+func (uva unlockedVestingAccount) TrackUndelegation(amount sdk.Coins) {
 }
 
-func (uva *unlockedVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins {
+func (uva unlockedVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins {
 	return sdk.NewCoins()
 }
 
-func (uva *unlockedVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins {
+func (uva unlockedVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins {
 	return sdk.NewCoins()
 }
 
-func (uva *unlockedVestingAccount) GetStartTime() int64 {
+func (uva unlockedVestingAccount) GetStartTime() int64 {
 	return 0
 }
 
-func (uva *unlockedVestingAccount) GetEndTime() int64 {
+func (uva unlockedVestingAccount) GetEndTime() int64 {
 	return 0
 }
 
-func (uva *unlockedVestingAccount) GetOriginalVesting() sdk.Coins {
+func (uva unlockedVestingAccount) GetOriginalVesting() sdk.Coins {
 	return sdk.NewCoins()
 }
 
-func (uva *unlockedVestingAccount) GetDelegatedFree() sdk.Coins {
+func (uva unlockedVestingAccount) GetDelegatedFree() sdk.Coins {
 	return sdk.NewCoins()
 }
 
-func (uva *unlockedVestingAccount) GetDelegatedVesting() sdk.Coins {
+func (uva unlockedVestingAccount) GetDelegatedVesting() sdk.Coins {
 	return sdk.NewCoins()
 }