Skip to content

Commit

Permalink
feat(pkg/std): ensure files are sorted in a MemPackage (#1618)
Browse files Browse the repository at this point in the history
<!-- please provide a detailed description of the changes made in this
pull request. -->
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

<details><summary>Contributors' checklist...</summary>

- [ ] 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
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Co-authored-by: Morgan Bazalgette <morgan@morganbaz.com>
  • Loading branch information
waymobetta and thehowl committed Mar 21, 2024
1 parent 7596f42 commit 41ff374
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 11 deletions.
1 change: 1 addition & 0 deletions gno.land/pkg/sdk/vm/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) error {
if err != nil {
return err
}

// Parse and run the files, construct *PV.
msgCtx := stdlibs.ExecContext{
ChainID: ctx.ChainID(),
Expand Down
38 changes: 27 additions & 11 deletions tm2/pkg/std/memfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ package std
import (
"fmt"
"regexp"
"sort"
"strings"

"github.com/gnolang/gno/tm2/pkg/errors"
)

type MemFile struct {
Expand Down Expand Up @@ -50,22 +49,39 @@ var (
// file names must contain dots.
// NOTE: this is to prevent conflicts with nested paths.
func (mempkg *MemPackage) Validate() error {
// add assertion that MemPkg contains at least 1 file
if len(mempkg.Files) <= 0 {
return fmt.Errorf("no files found within package %q", mempkg.Name)
}

if !rePkgName.MatchString(mempkg.Name) {
return errors.New(fmt.Sprintf("invalid package name %q, failed to match %q", mempkg.Name, rePkgName))
return fmt.Errorf("invalid package name %q, failed to match %q", mempkg.Name, rePkgName)
}
if !rePkgOrRlmPath.MatchString(mempkg.Path) {
return errors.New(fmt.Sprintf("invalid package/realm path %q, failed to match %q", mempkg.Path, rePkgOrRlmPath))
return fmt.Errorf("invalid package/realm path %q, failed to match %q", mempkg.Path, rePkgOrRlmPath)
}
fnames := map[string]struct{}{}
for _, memfile := range mempkg.Files {
if !reFileName.MatchString(memfile.Name) {
return errors.New(fmt.Sprintf("invalid file name %q, failed to match %q", memfile.Name, reFileName))
// enforce sorting files based on Go conventions for predictability
sorted := sort.SliceIsSorted(
mempkg.Files,
func(i, j int) bool {
return mempkg.Files[i].Name < mempkg.Files[j].Name
},
)
if !sorted {
return fmt.Errorf("mempackage %q has unsorted files", mempkg.Path)
}

var prev string
for i, file := range mempkg.Files {
if !reFileName.MatchString(file.Name) {
return fmt.Errorf("invalid file name %q, failed to match %q", file.Name, reFileName)
}
if _, exists := fnames[memfile.Name]; exists {
return errors.New(fmt.Sprintf("duplicate file name %q", memfile.Name))
if i > 0 && prev == file.Name {
return fmt.Errorf("duplicate file name %q", file.Name)
}
fnames[memfile.Name] = struct{}{}
prev = file.Name
}

return nil
}

Expand Down
53 changes: 53 additions & 0 deletions tm2/pkg/std/memfile_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package std

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestMemPackage_Validate(t *testing.T) {
tt := []struct {
name string
mpkg *MemPackage
errContains string
}{
{
"Correct",
&MemPackage{
Name: "hey",
Path: "gno.land/r/demo/hey",
Files: []*MemFile{{Name: "a.gno"}},
},
"",
},
{
"Unsorted",
&MemPackage{
Name: "hey",
Path: "gno.land/r/demo/hey",
Files: []*MemFile{{Name: "b.gno"}, {Name: "a.gno"}},
},
`mempackage "gno.land/r/demo/hey" has unsorted files`,
},
{
"Duplicate",
&MemPackage{
Name: "hey",
Path: "gno.land/r/demo/hey",
Files: []*MemFile{{Name: "a.gno"}, {Name: "a.gno"}},
},
`duplicate file name "a.gno"`,
},
}
for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
err := tc.mpkg.Validate()
if tc.errContains == "" {
assert.NoError(t, err)
} else {
assert.ErrorContains(t, err, tc.errContains)
}
})
}
}

0 comments on commit 41ff374

Please sign in to comment.