refactor: improve docs, comments, tests, and small refactors #303
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ import ( | |
| "encoding/json" | ||
| "flag" | ||
| "fmt" | ||
| "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) | ||
|
|
||
| // 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) | ||
| 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 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 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 | ||
| // 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{} | ||
| preCliffAmount := sdk.NewCoins() | ||
| i := 0 | ||
| for ; i < len(events) && !events[i].Time.After(cliff); i++ { | ||
| preCliffAmount = preCliffAmount.Add(events[i].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 a schedule file from stdin and writes a sequence of vesting | ||
| // events in local time to stdout. See README.md for the format. | ||
|
There was a problem hiding this comment. In local time? Oof. If that's the case we should probably find a way to convey UTC offsets. There was a problem hiding this comment. I think it matches the anticipated usage pattern. The comment about timezones in |
||
| func readCmd() { | ||
| 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 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 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 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 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 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) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON implies text. However, from all the format-related comments, it could be confusing to have it spread out. I've added a single format section with an example that I hope makes it more clear.