feat(pkg/std): ensure files are sorted in a MemPackage

[waymobetta]

This PR ensures in MemPackage.Validate that the files that it is being passed are correctly sorted. This is called in many places, among which the VM keeper.

Context:
It was discovered that during the add package process, gno files get processed in an unpredictable order.

This relates to the below related issue (#1482) regarding multiple init statements being allowed since these init statements would potentially be processed in unknown order.

Related: #1482

Contributors' checklist...

• Added new tests, or not needed, or not feasible
• Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
• Updated the official documentation or not needed
• No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
• Added references to related issues and PRs
• Provided any useful hints for running manual tests
• Added new benchmarks to generated graphs, if any. More info here.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.49%. Comparing base (7596f42) to head (9e8b415).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1618      +/-   ##
==========================================
+ Coverage   44.79%   47.49%   +2.70%     
==========================================
  Files         459      388      -71     
  Lines       67642    61304    -6338     
==========================================
- Hits        30300    29117    -1183     
+ Misses      34808    29749    -5059     
+ Partials     2534     2438      -96     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[deelawn]

I think perhaps the only change necessary is to add something like this:

sort.Slice(
    msg.Package.Files,
    func(i, j int) bool {
        return msg.Package.Files[i].Name < msg.Package.Files[j].Name
    },
)

That should sort the package files in place as long as the original order doesn't need to be preserved, but I don't think it does.

[waymobetta]

I think perhaps the only change necessary is to add something like this:

sort.Slice(
    msg.Package.Files,
    func(i, j int) bool {
        return msg.Package.Files[i].Name < msg.Package.Files[j].Name
    },
)

That should sort the package files in place as long as the original order doesn't need to be preserved, but I don't think it does.

I was thinking copy and replace versus altering in place for safety but this is a significantly more elegant solution.

[deelawn]

Looks good to me. One more thing to maybe consider -- this seems to be the correct way to sort strings, but does is the sorting exactly the same as what happens when go parses files? Probably. But how do we know for sure?

[thehowl]

I think to address your comment about ordering and doing a small performance improvement, too:

• Move the sorting into std.MemPackage.Validate (this will eventually be moved to sdk/vm, per this comment discussed with Jae)
• After sorting, change the current check for duplicates in Validate() (the one using a map) to a simpler one:

// NOTE: Before this, add assertion that MemPkg contains at least 1 file

prev := mempkg.Files[0].Name
for _, file := range mempkg.Files[1:] {
	if prev == file.Name {
		// return error
	}
	prev = file.Name
}

(The following works because the slice is sorted)

[waymobetta]

I think to address your comment about ordering and doing a small performance improvement, too:

• Move the sorting into std.MemPackage.Validate (this will eventually be moved to sdk/vm, per this comment discussed with Jae)
• After sorting, change the current check for duplicates in Validate() (the one using a map) to a simpler one:

// XXX: add assertion that MemPkg contains at least 1 file
prev := mempkg.Files[0].Name
for _, file := range mempkg.Files[1:] {
	if prev == file.Name {
		// return error
	}
	prev = file.Name
}

(The following works because the slice is sorted)

@thehowl Change to your suggestion or merge as is based on previous approvals?

[thehowl]

@waymobetta change it to mine, we get to kill two birds with one stone

[thehowl]

I fixed up this PR to move it to validate that the files are sorted instead. This seems more coherent with the line of thought of Validate() in general (ie. being readonly); rejecting unsorted MemPackage seems reasonable as there is currently no way to craft a MemPackage through official tools (like gnokey) where the filenames are not sorted.

I'll wait for another review, as my changes significantly change Jon's original work.

[deelawn]

Seems reasonable. If we start seeing transactions failing because of unsorted packages then we can take a look to see what's going on.