Merge pull request #303 from agoric-labs/jimlarson-vestcalc-cleanup

refactor: improve docs, comments, tests, and small refactors

x/auth/vesting/cmd/vestcalc/README.md

@@ -30,26 +30,63 @@ Run `go install` in this directory, which will create or update the
 [documentation](https://pkg.go.dev/cmd/go) for the `go` command-line
 tool for other options.
 
+## Schedule format
+
+Schedules are expressed in the format expected by the `create-periodic-vesting-account`
+or `create-clawback-vesting-account` cli commands, namely a JSON
+object with the members:
+
+- `"start_time"`: integer UNIX time;
+- `"periods"`: an array of objects describing vesting events with members:
+    - "coins": string giving the text coins format for the additional amount vested by this event;
+    - "length_seconds": positive integer seconds from the last vesting event, or from the start time for the first vesting event.
+
+For instance:
+
+```
+{
+  "start_time": 1700000000,
+  "periods": [
+    {
+      "coins": "10000000uatom,500000000ubld",
+      "length_seconds": 2678400
+    },
+    {
+      "coins": "500000000ubld",
+      "length_seconds": 31536000
+    }
+  ]
+}
+
+```
+
 ## Writing a schedule
 
-When the `--write` flag is set, the tool will write a schedule in JSON to
-stdout. The following flags control the output:
+When the `--write` flag is set, the tool will write a schedule to stdout.
+The following flags control the output:
 
-- `--coins:` The coins to vest, e.g. `100ubld,50urun`.
-- `--months`: The number of months to vest over.
-- `--time`: The time of day of the vesting event, in 24-hour HH:MM format.
-  Defaults to midnight.
+- `--coins:` The total coins to vest, e.g. `100ubld,50urun`.
+- `--months`: The total number of months to complete vesting.
 - `--start`: The vesting start time: i.e. the first event happens in the
   next month. Specified in the format `YYYY-MM-DD` or `YYYY-MM-DDThh:mm`,
   e.g. `2006-01-02T15:04` for 3:04pm on January 2, 2006.
-- `--cliffs`: One or more vesting cliffs in `YYYY-MM-DD` or `YYYY-MM-DDThh:mm`
+- `--time`: The time of day (in the local timezone) of the vesting events, in 24-hour HH:MM format.
+  Defaults to midnight.
+- `--cliffs`: Vesting cliffs in `YYYY-MM-DD` or `YYYY-MM-DDThh:mm`
   format. Only the latest one will have any effect, but it is useful to let
   the computer do that calculation to avoid mistakes. Multiple cliff dates
-  can be separated by commas or given as multiple arguments.
+  can be separated by commas or given as multiple arguments. Cliffs are not required.
+
+The vesting events will occur each month following the start time on the same
+day of the month (or the last day of the month, if the month does not have a
+sufficient number of days), for the specified number of months. The total coins
+to vest will be divided as evenly as possible among all the vesting events.
+Lastly, all events before the last cliff time, if any, are consolidated into a single event
+at the last cliff time with the sum of the event amounts.
 
 ## Reading a schedule
 
-When the `--read` flag is set, the tool will read a schedule in JSON from
+When the `--read` flag is set, the tool will read a schedule from
 stdin and write the vesting events in absolute time to stdout.
 
 ## Examples

x/auth/vesting/cmd/vestcalc/vestcalc.go

@@ -4,7 +4,7 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"os"
 	"strings"
 	"time"
@@ -13,6 +13,9 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/auth/vesting/client/cli"
 )
 
+// vestcalc is a utility for creating or reading schedule files
+// for use in some vesting account types.  See README.md for usage.
+
 // divide returns the division of total as evenly as possible.
 // Divisor must be 1 or greater and total must be nonnegative.
 func divide(total sdk.Int, divisor int) ([]sdk.Int, error) {
@@ -24,17 +27,29 @@ func divide(total sdk.Int, divisor int) ([]sdk.Int, error) {
 		return nil, fmt.Errorf("total must be nonnegative")
 	}
 	divisions := make([]sdk.Int, divisor)
-	// Calculate truncated division and the remainder.
-	// Fact: remainder < divisions, hence fits in int64
+
+	// Ideally we could compute total of the first i divisions as
+	//     cumulative(i) = floor((total * i) / divisor)
+	// and so
+	//     divisions[i] = cumulative(i + 1) - cumulative(i)
+	// but this could lead to numeric overflow for large values of total.
+	// Instead, we'll compute
+	//     truncated = floor(total / divisor)
+	// so that
+	//     total = truncated * divisor + remainder
+	// where remainder < divisor, then divide the remainder via the
+	// above algorithm - which now won't overflow - and sum the
+	// truncated and slices of the remainder to form the divisions.
 	truncated := total.QuoRaw(div64)
 	remainder := total.ModRaw(div64)
-	fraction := sdk.NewInt(0) // portion of remainder which has been doled out
+	cumulative := sdk.NewInt(0) // portion of remainder which has been doled out
 	for i := int64(0); i < div64; i++ {
-		// multiply will not overflow since remainder and i are < 2^63
-		nextFraction := remainder.MulRaw(i + 1).QuoRaw(div64)
-		divisions[i] = truncated.Add(nextFraction.Sub(fraction))
-		fraction = nextFraction
+		// multiply will not overflow since remainder and div64 are < 2^63
+		nextCumulative := remainder.MulRaw(i + 1).QuoRaw(div64)
+		divisions[i] = truncated.Add(nextCumulative.Sub(cumulative))
+		cumulative = nextCumulative
 	}
+
 	// Integrity check
 	sum := sdk.NewInt(0)
 	for _, x := range divisions {
@@ -46,6 +61,8 @@ func divide(total sdk.Int, divisor int) ([]sdk.Int, error) {
 	return divisions, nil
 }
 
+// divideCoins divides the coins into divisor separate parts as evenly as possible.
+// Divisor must be positive. Returns an array holding the division.
 func divideCoins(coins sdk.Coins, divisor int) ([]sdk.Coins, error) {
 	if divisor < 1 {
 		return nil, fmt.Errorf("divisor must be 1 or greater")
@@ -81,6 +98,7 @@ func divideCoins(coins sdk.Coins, divisor int) ([]sdk.Coins, error) {
 // monthlyVestTimes generates timestamps for successive months after startTime.
 // The monthly events occur at the given time of day. If the month is not
 // long enough for the desired date, the last day of the month is used.
+// The number of months must be positive.
 func monthlyVestTimes(startTime time.Time, months int, timeOfDay time.Time) ([]time.Time, error) {
 	if months < 1 {
 		return nil, fmt.Errorf("must have at least one vesting period")
@@ -101,6 +119,7 @@ func monthlyVestTimes(startTime time.Time, months int, timeOfDay time.Time) ([]t
 		times[i-1] = time.Date(tm.Year(), tm.Month(), tm.Day(), hour, minute, second, 0, location)
 	}
 	// Integrity check: dates must be sequential and 26-33 days apart.
+	// (Jan 31 to Feb 28 or Feb 28 to Mar 31, plus slop for DST.)
 	lastTime := startTime
 	for _, tm := range times {
 		duration := tm.Sub(lastTime)
@@ -115,7 +134,7 @@ func monthlyVestTimes(startTime time.Time, months int, timeOfDay time.Time) ([]t
 	return times, nil
 }
 
-// marshalVestingData gives the JSON encoding.
+// marshalVestingData writes the vesting data as JSON.
 func marshalVestingData(data cli.VestingData) ([]byte, error) {
 	return json.MarshalIndent(data, "", "  ")
 }
@@ -133,7 +152,8 @@ type event struct {
 	Coins sdk.Coins
 }
 
-// zipEvents generates events by zipping corresponding amounts and times.
+// zipEvents generates events by zipping corresponding amounts and times
+// from equal-sized arrays, returning an event array of the same size.
 func zipEvents(divisions []sdk.Coins, times []time.Time) ([]event, error) {
 	n := len(divisions)
 	if len(times) != n {
@@ -166,24 +186,19 @@ func marshalEvents(events []event) ([]byte, error) {
 // into a single event, leaving subsequent events unchanged.
 func applyCliff(events []event, cliff time.Time) ([]event, error) {
 	newEvents := []event{}
-	coins := sdk.NewCoins()
-	for _, e := range events {
-		if !e.Time.After(cliff) {
-			coins = coins.Add(e.Coins...)
-			continue
-		}
-		if !coins.IsZero() {
-			cliffEvent := event{Time: cliff, Coins: coins}
-			newEvents = append(newEvents, cliffEvent)
-			coins = sdk.NewCoins()
-		}
-		newEvents = append(newEvents, e)
+	preCliffAmount := sdk.NewCoins()
+	i := 0
+	for ; i < len(events) && !events[i].Time.After(cliff); i++ {
+		preCliffAmount = preCliffAmount.Add(events[i].Coins...)
 	}
-	if !coins.IsZero() {
-		// special case if all events are before the cliff
-		cliffEvent := event{Time: cliff, Coins: coins}
+	if !preCliffAmount.IsZero() {
+		cliffEvent := event{Time: cliff, Coins: preCliffAmount}
 		newEvents = append(newEvents, cliffEvent)
 	}
+	for ; i < len(events); i++ {
+		newEvents = append(newEvents, events[i])
+	}
+
 	// integrity check
 	oldTotal := sdk.NewCoins()
 	for _, e := range events {
@@ -196,6 +211,7 @@ func applyCliff(events []event, cliff time.Time) ([]event, error) {
 	if !oldTotal.IsEqual(newTotal) {
 		return nil, fmt.Errorf("applying vesting cliff changed total from %s to %s", oldTotal, newTotal)
 	}
+
 	return newEvents, nil
 }
 
@@ -335,6 +351,8 @@ const hhmmFmt = "15:04"
 // isoDate is time.Time as a flag.Value in shortIsoFmt.
 type isoDate struct{ time.Time }
 
+var _ flag.Value = &isoDate{}
+
 // Set implements flag.Value.Set().
 func (id *isoDate) Set(s string) error {
 	t, err := parseIso(s)
@@ -360,6 +378,8 @@ func isoDateFlag(name string, usage string) *time.Time {
 // isoDateList is []time.Time as a flag.Value in repeated or comma-separated shortIsoFmt.
 type isoDateList []time.Time
 
+var _ flag.Value = &isoDateList{}
+
 // Set implements flag.Value.Set().
 func (dates *isoDateList) Set(s string) error {
 	for _, ds := range strings.Split(s, ",") {
@@ -395,6 +415,8 @@ func isoDateListFlag(name string, usage string) *isoDateList {
 // isoTime is time.Time as a flagValue in HH:MM format.
 type isoTime struct{ time.Time }
 
+var _ flag.Value = &isoTime{}
+
 // Set implements flag.Value.Set().
 func (it *isoTime) Set(s string) error {
 	t, err := time.Parse(hhmmFmt, s)
@@ -431,10 +453,10 @@ var (
 	flagWrite  = flag.Bool("write", false, "Write periods file to stdout.")
 )
 
-// readCmd reads periods in JSON from stdin and writes a sequence of vesting
-// events in local time to stdout.
+// readCmd reads a schedule file from stdin and writes a sequence of vesting
+// events in local time to stdout. See README.md for the format.
 func readCmd() {
-	bzIn, err := ioutil.ReadAll(os.Stdin)
+	bzIn, err := io.ReadAll(os.Stdin)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "cannot read stdin: %v", err)
 		return
@@ -458,7 +480,9 @@ func readCmd() {
 
 // writeConfig bundles data needed for the write operation.
 type writeConfig struct {
-	Coins     sdk.Coins
+	// Coins is the total amount to be vested.
+	Coins sdk.Coins
+	// Months is the number of months to vest over. Must be positive.
 	Months    int
 	TimeOfDay time.Time
 	Start     time.Time
@@ -483,9 +507,7 @@ func genWriteConfig() (writeConfig, error) {
 	return wc, nil
 }
 
-// generateEvents generates vesting events for the given amount and
-// denomination across the given monthly vesting events with the given start
-// time and subject to the vesting cliff times, if any.
+// generateEvents generates vesting events from the writeConfig.
 func (wc writeConfig) generateEvents() ([]event, error) {
 	divisions, err := divideCoins(wc.Coins, wc.Months)
 	if err != nil {
@@ -509,13 +531,13 @@ func (wc writeConfig) generateEvents() ([]event, error) {
 	return events, nil
 }
 
-// convertRelative converts absolute-time events to relative periods.
+// convertRelative converts absolute-time events to VestingData relative to the Start time.
 func (wc writeConfig) convertRelative(events []event) cli.VestingData {
 	return eventsToVestingData(wc.Start, events)
 }
 
-// writeCmd generates a set of vesting events based on flags and writes a
-// sequences of periods in JSON format to stdout.
+// writeCmd generates a set of vesting events based on parsed flags
+// and writes a schedule file to stdout.
 func writeCmd() {
 	wc, err := genWriteConfig()
 	if err != nil {
@@ -536,7 +558,8 @@ func writeCmd() {
 	fmt.Println(string(bz))
 }
 
-// main executes either readCmd() or writeCmd() based on flags.
+// main parses the flags and executes a subcommand based on flags.
+// See README.md for flags and subcommands.
 func main() {
 	flag.Parse()
 	switch {
@@ -547,5 +570,6 @@ func main() {
 	default:
 		fmt.Fprintln(os.Stderr, "Must specify one of --read or --write")
 		flag.Usage()
+		os.Exit(1)
 	}
 }

x/auth/vesting/cmd/vestcalc/vestcalc_test.go

@@ -183,6 +183,26 @@ func TestMonthlyVestTimes(t *testing.T) {
 				iso("2021-01-01T12:00"),
 			},
 		},
+		{
+			Name:      "clip to end of month",
+			Start:     iso("2021-01-31"),
+			Months:    12,
+			TimeOfDay: hhmm("17:00"),
+			Want: []time.Time{
+				iso("2021-02-28T17:00"),
+				iso("2021-03-31T17:00"),
+				iso("2021-04-30T17:00"),
+				iso("2021-05-31T17:00"),
+				iso("2021-06-30T17:00"),
+				iso("2021-07-31T17:00"),
+				iso("2021-08-31T17:00"),
+				iso("2021-09-30T17:00"),
+				iso("2021-10-31T17:00"),
+				iso("2021-11-30T17:00"),
+				iso("2021-12-31T17:00"),
+				iso("2022-01-31T17:00"),
+			},
+		},
 	} {
 		t.Run(tt.Name, func(t *testing.T) {
 			got, err := monthlyVestTimes(tt.Start, tt.Months, tt.TimeOfDay)