From b20e2da3da95dfd315f39884d4a029a6227a0705 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 20 Feb 2019 19:07:15 +0100 Subject: [PATCH 01/22] blog errors eliminated --- errors/errors.go | 6 ++ examples/tutorial/x/blog/errors.go | 86 ++--------------------- examples/tutorial/x/blog/handlers.go | 30 ++++---- examples/tutorial/x/blog/handlers_test.go | 45 ++++++------ examples/tutorial/x/blog/models.go | 18 ++--- examples/tutorial/x/blog/msgs.go | 21 +++--- 6 files changed, 70 insertions(+), 136 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index 182061a1..ce57adb4 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -57,6 +57,12 @@ var ( // ErrInvalidAmount stands for invalid amount of whatever ErrInvalidAmount = Register(13, "invalid amount") + // ErrInvalidInput stands for general input problems indication + ErrInvalidInput = Register(14, "invalid input") + + // ErrExists already exists error + ErrExists = Register(15, "already exists") + // ErrPanic is only set when we recover from a panic, so we know to redact potentially sensitive system info ErrPanic = Register(111222, "panic") ) diff --git a/examples/tutorial/x/blog/errors.go b/examples/tutorial/x/blog/errors.go index 4bb1aac2..ba01109c 100644 --- a/examples/tutorial/x/blog/errors.go +++ b/examples/tutorial/x/blog/errors.go @@ -9,25 +9,17 @@ import ( // ABCI Response Codes // tutorial reserves 400 ~ 420. const ( - CodeInvalidText uint32 = 400 - CodeInvalidAuthor uint32 = 401 CodeNegativeNumber uint32 = 402 CodeInvalidBlog uint32 = 403 ) var ( - errInvalidTitle = fmt.Errorf("Title is too long or too short") - errInvalidText = fmt.Errorf("Text is too long or too short") - errInvalidName = fmt.Errorf("Name is too long") - errDescriptionTooLong = fmt.Errorf("Description is too long") - - errNoAuthor = fmt.Errorf("No author for post") - errInvalidAuthorCount = fmt.Errorf("Invalid number of blog authors") - errUnauthorisedBlogAuthor = fmt.Errorf("Unauthorised blog author") - errUnauthorisedPostAuthor = fmt.Errorf("Unauthorised post author") - errUnauthorisedProfileAuthor = fmt.Errorf("Unauthorised profile author") - errAuthorNotFound = fmt.Errorf("Author not found") - errAuthorAlreadyExist = fmt.Errorf("Author already exists") + invalidTitle = "title is too long or too short" + invalidText = "text is too long or too short" + invalidName = "name is too long" + descriptionTooLong = "description is too long" + unauthorisedBlogAuthor = "blog author %X" + unauthorisedPostAuthor = "post author %X" errNegativeArticles = fmt.Errorf("Article count is negative") errNegativeCreation = fmt.Errorf("Creation block is negative") @@ -37,72 +29,6 @@ var ( errBlogOneAuthorLeft = fmt.Errorf("Unable to remove last blog author") ) -func ErrInvalidTitle() error { - return errors.WithCode(errInvalidTitle, CodeInvalidText) -} -func ErrInvalidText() error { - return errors.WithCode(errInvalidText, CodeInvalidText) -} -func ErrInvalidName() error { - return errors.WithCode(errInvalidName, CodeInvalidText) -} -func ErrDescriptionTooLong() error { - return errors.WithCode(errDescriptionTooLong, CodeInvalidText) -} -func IsInvalidTextError(err error) bool { - return errors.HasErrorCode(err, CodeInvalidText) -} - -func ErrNoAuthor() error { - return errors.WithCode(errNoAuthor, CodeInvalidAuthor) -} -func ErrInvalidAuthorCount(count int) error { - msg := fmt.Sprintf("authors=%d", count) - return errors.WithLog(msg, errInvalidAuthorCount, CodeInvalidAuthor) -} -func ErrUnauthorisedBlogAuthor(author []byte) error { - msg := fmt.Sprintf("author=%X", author) - return errors.WithLog(msg, errUnauthorisedBlogAuthor, CodeInvalidAuthor) -} -func ErrUnauthorisedPostAuthor(author []byte) error { - msg := fmt.Sprintf("author=%X", author) - return errors.WithLog(msg, errUnauthorisedPostAuthor, CodeInvalidAuthor) -} -func ErrUnauthorisedProfileAuthor(author []byte) error { - msg := fmt.Sprintf("author=%X", author) - return errors.WithLog(msg, errUnauthorisedPostAuthor, CodeInvalidAuthor) -} -func ErrAuthorNotFound(author []byte) error { - msg := fmt.Sprintf("author=%X", author) - return errors.WithLog(msg, errAuthorNotFound, CodeInvalidAuthor) -} -func ErrAuthorAlreadyExist(author []byte) error { - msg := fmt.Sprintf("author=%X", author) - return errors.WithLog(msg, errAuthorAlreadyExist, CodeInvalidAuthor) -} -func IsInvalidAuthorError(err error) bool { - return errors.HasErrorCode(err, CodeInvalidAuthor) -} - -func ErrNegativeArticles() error { - return errors.WithCode(errNegativeArticles, CodeNegativeNumber) -} -func ErrNegativeCreation() error { - return errors.WithCode(errNegativeCreation, CodeNegativeNumber) -} -func IsNegativeNumberError(err error) bool { - return errors.HasErrorCode(err, CodeNegativeNumber) -} - -func ErrBlogNotFound() error { - return errors.WithCode(errBlogNotFound, CodeInvalidBlog) -} -func ErrBlogExist() error { - return errors.WithCode(errBlogExist, CodeInvalidBlog) -} func ErrBlogOneAuthorLeft() error { return errors.WithCode(errBlogOneAuthorLeft, CodeInvalidBlog) } -func IsInvalidBlogError(err error) bool { - return errors.HasErrorCode(err, CodeInvalidBlog) -} diff --git a/examples/tutorial/x/blog/handlers.go b/examples/tutorial/x/blog/handlers.go index b8d2a9c9..72cacfa3 100644 --- a/examples/tutorial/x/blog/handlers.go +++ b/examples/tutorial/x/blog/handlers.go @@ -90,7 +90,7 @@ func (h CreateBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, ErrUnauthorisedBlogAuthor(nil) + return nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil) } msg, err := tx.GetMsg() @@ -111,7 +111,7 @@ func (h CreateBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Check the blog does not already exist obj, err := h.bucket.Get(db, []byte(createBlogMsg.Slug)) if err != nil || (obj != nil && obj.Value() != nil) { - return nil, ErrBlogExist() + return nil, errors.ErrExists.New("blog") } return createBlogMsg, nil @@ -184,7 +184,7 @@ func (h CreatePostMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Check the author is one of the Tx signer if !h.auth.HasAddress(ctx, createPostMsg.Author) { - return nil, nil, ErrUnauthorisedPostAuthor(createPostMsg.Author) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, createPostMsg.Author) } err = createPostMsg.Validate() @@ -198,7 +198,7 @@ func (h CreatePostMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w return nil, nil, err } if obj == nil || (obj != nil && obj.Value() == nil) { - return nil, nil, ErrBlogNotFound() + return nil, nil, errors.ErrNotFound.New("blog") } blog := obj.Value().(*Blog) @@ -252,7 +252,7 @@ func (h RenameBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, nil, ErrUnauthorisedBlogAuthor(nil) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil) } msg, err := tx.GetMsg() @@ -277,13 +277,13 @@ func (h RenameBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w return nil, nil, err } if obj == nil || (obj != nil && obj.Value() == nil) { - return nil, nil, ErrBlogNotFound() + return nil, nil, errors.ErrNotFound.New("blog") } blog := obj.Value().(*Blog) // Check main signer is one of the blog authors if findAuthor(blog.Authors, sender.Address()) == -1 { - return nil, nil, ErrUnauthorisedBlogAuthor(sender.Address()) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, sender.Address()) } return renameBlogMsg, blog, nil @@ -345,7 +345,7 @@ func (h ChangeBlogAuthorsMsgHandler) validate(ctx weave.Context, db weave.KVStor // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, nil, ErrUnauthorisedBlogAuthor(nil) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil) } msg, err := tx.GetMsg() @@ -369,13 +369,13 @@ func (h ChangeBlogAuthorsMsgHandler) validate(ctx weave.Context, db weave.KVStor return nil, nil, err } if obj == nil || (obj != nil && obj.Value() == nil) { - return nil, nil, ErrBlogNotFound() + return nil, nil, errors.ErrNotFound.New("blog") } blog := obj.Value().(*Blog) // Check main signer is one of the blog authors if findAuthor(blog.Authors, sender.Address()) == -1 { - return nil, nil, ErrUnauthorisedBlogAuthor(sender.Address()) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, sender.Address()) } // Get the author index @@ -383,7 +383,7 @@ func (h ChangeBlogAuthorsMsgHandler) validate(ctx weave.Context, db weave.KVStor if changeBlogAuthorsMsg.Add { // When removing an author we must ensure it does not exist already if authorIdx >= 0 { - return nil, nil, ErrAuthorAlreadyExist(changeBlogAuthorsMsg.Author) + return nil, nil, errors.ErrExists.Newf("author: %X", changeBlogAuthorsMsg.Author) } } else { // When removing an author we must ensure : @@ -391,11 +391,11 @@ func (h ChangeBlogAuthorsMsgHandler) validate(ctx weave.Context, db weave.KVStor // 2 - There will be at least one other author left if authorIdx == -1 { - return nil, nil, ErrAuthorNotFound(changeBlogAuthorsMsg.Author) + return nil, nil, errors.ErrNotFound.Newf("author: %X", changeBlogAuthorsMsg.Author) } if len(blog.Authors) == 1 { - return nil, nil, ErrBlogOneAuthorLeft() + return nil, nil, errors.ErrInvalidState.New("one author left") } } @@ -452,7 +452,7 @@ func (h SetProfileMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, nil, ErrUnauthorisedBlogAuthor(nil) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil) } msg, err := tx.GetMsg() @@ -468,7 +468,7 @@ func (h SetProfileMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // if author is here we use it for authentication if setProfileMsg.Author != nil { if !h.auth.HasAddress(ctx, setProfileMsg.Author) { - return nil, nil, ErrUnauthorisedProfileAuthor(setProfileMsg.Author) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, setProfileMsg.Author) } } diff --git a/examples/tutorial/x/blog/handlers_test.go b/examples/tutorial/x/blog/handlers_test.go index ffefc530..2929c837 100644 --- a/examples/tutorial/x/blog/handlers_test.go +++ b/examples/tutorial/x/blog/handlers_test.go @@ -171,7 +171,8 @@ func testHandlerCheck(t *testing.T, testcases []testcase) { require.EqualValues(t, test.Res, res, test.Name) } else { require.Error(t, err, test.Name) // to avoid seg fault at the next line - require.EqualError(t, err, test.Err.Error(), test.Name) + errors.Is(err, test.Err) + require.True(t, errors.Is(err, test.Err), test.Name) } } } @@ -228,7 +229,7 @@ func TestCreateBlogMsgHandlerCheck(t *testing.T) { }, { Name: "no authors", - Err: ErrInvalidAuthorCount(0), + Err: errors.ErrInvalidState.Newf("authors: %d", 0), Handler: createBlogMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreateBlogMsg{ @@ -238,7 +239,7 @@ func TestCreateBlogMsgHandlerCheck(t *testing.T) { }, { Name: "no slug", - Err: ErrInvalidName(), + Err: errors.ErrInvalidInput.New(invalidName), Handler: createBlogMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreateBlogMsg{ @@ -247,7 +248,7 @@ func TestCreateBlogMsgHandlerCheck(t *testing.T) { }, { Name: "no title", - Err: ErrInvalidTitle(), + Err: errors.ErrInvalidInput.New(invalidTitle), Handler: createBlogMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreateBlogMsg{ @@ -256,7 +257,7 @@ func TestCreateBlogMsgHandlerCheck(t *testing.T) { }, { Name: "no signer", - Err: ErrUnauthorisedBlogAuthor(nil), + Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil), Handler: createBlogMsgHandlerFn, Msg: &CreateBlogMsg{ Slug: "this_is_a_blog", @@ -266,7 +267,7 @@ func TestCreateBlogMsgHandlerCheck(t *testing.T) { }, { Name: "creating twice", - Err: ErrBlogExist(), + Err: errors.ErrExists.New("blog"), Handler: createBlogMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreateBlogMsg{ @@ -389,7 +390,7 @@ func TestCreatePostMsgHandlerCheck(t *testing.T) { }, { Name: "no title", - Err: ErrInvalidTitle(), + Err: errors.ErrInvalidInput.New(invalidTitle), Handler: createPostMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreatePostMsg{ @@ -400,7 +401,7 @@ func TestCreatePostMsgHandlerCheck(t *testing.T) { }, { Name: "no text", - Err: ErrInvalidText(), + Err: errors.ErrInvalidInput.New(invalidText), Handler: createPostMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreatePostMsg{ @@ -411,7 +412,7 @@ func TestCreatePostMsgHandlerCheck(t *testing.T) { }, { Name: "no author", - Err: ErrUnauthorisedPostAuthor(nil), + Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, nil), Handler: createPostMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreatePostMsg{ @@ -422,7 +423,7 @@ func TestCreatePostMsgHandlerCheck(t *testing.T) { }, { Name: "unauthorized", - Err: ErrUnauthorisedPostAuthor(unauthorised.Address()), + Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, unauthorised.Address()), Handler: createPostMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreatePostMsg{ @@ -445,7 +446,7 @@ func TestCreatePostMsgHandlerCheck(t *testing.T) { }, { Name: "missing blog dependency", - Err: ErrBlogNotFound(), + Err: errors.ErrNotFound.New("blog"), Handler: createPostMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreatePostMsg{ @@ -552,7 +553,7 @@ func TestRenameBlogMsgHandlerCheck(t *testing.T) { }, { Name: "no title", - Err: ErrInvalidTitle(), + Err: errors.ErrInvalidInput.New(invalidTitle), Handler: renameBlogMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &RenameBlogMsg{ @@ -561,7 +562,7 @@ func TestRenameBlogMsgHandlerCheck(t *testing.T) { }, { Name: "missing dependency", - Err: ErrBlogNotFound(), + Err: errors.ErrNotFound.New("blog"), Handler: renameBlogMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &RenameBlogMsg{ @@ -571,7 +572,7 @@ func TestRenameBlogMsgHandlerCheck(t *testing.T) { }, { Name: "no signer", - Err: ErrUnauthorisedBlogAuthor(nil), + Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil), Handler: renameBlogMsgHandlerFn, Msg: &RenameBlogMsg{ Slug: "this_is_a_blog", @@ -679,7 +680,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "adding existing author", - Err: ErrAuthorAlreadyExist(newAuthor.Address()), + Err: errors.ErrExists.Newf("author: %X", newAuthor.Address()), Handler: changeBlogAuthorsMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &ChangeBlogAuthorsMsg{ @@ -701,7 +702,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "removing unexisting author", - Err: ErrAuthorNotFound(newAuthor.Address()), + Err: errors.ErrNotFound.Newf("author: %X", newAuthor.Address()), Handler: changeBlogAuthorsMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &ChangeBlogAuthorsMsg{ @@ -723,7 +724,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "removing last author", - Err: ErrBlogOneAuthorLeft(), + Err: errors.ErrInvalidState.New(""), Handler: changeBlogAuthorsMsgHandlerFn, Perms: []weave.Condition{authorToRemove}, Msg: &ChangeBlogAuthorsMsg{ @@ -765,7 +766,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "adding with missing dep", - Err: ErrBlogNotFound(), + Err: errors.ErrNotFound.New("blog"), Handler: changeBlogAuthorsMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &ChangeBlogAuthorsMsg{ @@ -776,7 +777,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "removing with missing dep", - Err: ErrBlogNotFound(), + Err: errors.ErrNotFound.New("blog"), Handler: changeBlogAuthorsMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &ChangeBlogAuthorsMsg{ @@ -787,7 +788,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "unsigned tx", - Err: ErrUnauthorisedBlogAuthor(nil), + Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil), Handler: changeBlogAuthorsMsgHandlerFn, Msg: &ChangeBlogAuthorsMsg{ Slug: "this_is_a_blog", @@ -891,7 +892,7 @@ func TestSetProfileMsgHandlerCheck(t *testing.T) { }, { Name: "no name", - Err: ErrInvalidName(), + Err: errors.ErrInvalidInput.New(invalidName), Handler: SetProfileMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &SetProfileMsg{ @@ -900,7 +901,7 @@ func TestSetProfileMsgHandlerCheck(t *testing.T) { }, { Name: "unauthorized author", - Err: ErrUnauthorisedProfileAuthor(author.Address()), + Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, author.Address()), Handler: SetProfileMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &SetProfileMsg{ diff --git a/examples/tutorial/x/blog/models.go b/examples/tutorial/x/blog/models.go index 551643d3..1de7a2ba 100644 --- a/examples/tutorial/x/blog/models.go +++ b/examples/tutorial/x/blog/models.go @@ -14,13 +14,13 @@ var _ orm.CloneableData = (*Blog)(nil) // Validate enforces limits of title size and number of authors func (b *Blog) Validate() error { if len(b.Title) > MaxTitleLength { - return ErrInvalidTitle() + return errors.ErrInvalidInput.New(invalidTitle) } if len(b.Authors) > MaxAuthors || len(b.Authors) == 0 { - return ErrInvalidAuthorCount(len(b.Authors)) + return errors.ErrInvalidState.Newf("authors: %d", len(b.Authors)) } if b.NumArticles < 0 { - return ErrNegativeArticles() + return errors.ErrInvalidModel.Newf("negative articles") } return nil } @@ -45,16 +45,16 @@ var _ orm.CloneableData = (*Post)(nil) // Validate enforces limits of text and title size func (p *Post) Validate() error { if len(p.Title) > MaxTitleLength { - return ErrInvalidTitle() + return errors.ErrInvalidInput.New(invalidTitle) } if len(p.Text) > MaxTextLength { - return ErrInvalidText() + return errors.ErrInvalidInput.New(invalidText) } if len(p.Author) == 0 { - return ErrNoAuthor() + return errors.ErrEmpty.New("author") } if p.CreationBlock < 0 { - return ErrNegativeCreation() + return errors.ErrInvalidModel.Newf("negative creation") } return nil } @@ -77,10 +77,10 @@ var _ orm.CloneableData = (*Profile)(nil) // Validate enforces limits of text and title size func (p *Profile) Validate() error { if len(p.Name) > MaxNameLength { - return ErrInvalidName() + return errors.ErrInvalidInput.New(invalidName) } if len(p.Description) > MaxDescriptionLength { - return ErrDescriptionTooLong() + return errors.ErrInvalidInput.New(descriptionTooLong) } return nil } diff --git a/examples/tutorial/x/blog/msgs.go b/examples/tutorial/x/blog/msgs.go index 94f66aa5..21e72ad0 100644 --- a/examples/tutorial/x/blog/msgs.go +++ b/examples/tutorial/x/blog/msgs.go @@ -4,6 +4,7 @@ import ( "regexp" "github.com/iov-one/weave" + "github.com/iov-one/weave/errors" ) const ( @@ -41,15 +42,15 @@ func (CreateBlogMsg) Path() string { func (s *CreateBlogMsg) Validate() error { // validate the strings if !IsValidName(s.Slug) { - return ErrInvalidName() + return errors.ErrInvalidInput.New(invalidName) } if len(s.Title) < MinTitleLength || len(s.Title) > MaxTitleLength { - return ErrInvalidTitle() + return errors.ErrInvalidInput.New(invalidTitle) } // check the number of authors authors := len(s.Authors) if authors < MinAuthors || authors > MaxAuthors { - return ErrInvalidAuthorCount(authors) + return errors.ErrInvalidState.Newf("authors: %d", authors) } // and validate all of them are valid addresses for _, a := range s.Authors { @@ -71,10 +72,10 @@ func (RenameBlogMsg) Path() string { // Validate makes sure that this is sensible func (s *RenameBlogMsg) Validate() error { if !IsValidName(s.Slug) { - return ErrInvalidName() + return errors.ErrInvalidInput.New(invalidName) } if len(s.Title) < MinTitleLength || len(s.Title) > MaxTitleLength { - return ErrInvalidTitle() + return errors.ErrInvalidInput.New(invalidTitle) } return nil } @@ -104,13 +105,13 @@ func (CreatePostMsg) Path() string { // Validate makes sure that this is sensible func (s *CreatePostMsg) Validate() error { if !IsValidName(s.Blog) { - return ErrInvalidName() + return errors.ErrInvalidInput.New(invalidName) } if len(s.Title) < MinTitleLength || len(s.Title) > MaxTitleLength { - return ErrInvalidTitle() + return errors.ErrInvalidInput.New(invalidTitle) } if len(s.Text) < MinTextLength || len(s.Text) > MaxTextLength { - return ErrInvalidText() + return errors.ErrInvalidInput.New(invalidText) } // if an author is present, validate it is a valid address @@ -131,10 +132,10 @@ func (SetProfileMsg) Path() string { // Validate makes sure that this is sensible func (s *SetProfileMsg) Validate() error { if !IsValidName(s.Name) { - return ErrInvalidName() + return errors.ErrInvalidInput.New(invalidName) } if len(s.Description) > MaxDescriptionLength { - return ErrDescriptionTooLong() + return errors.ErrInvalidInput.New(descriptionTooLong) } // if an author is present, validate it is a valid address if len(s.Author) > 0 { From 4d0452427a255a9e9d076c1e3c4f430ffeea0f33 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 20 Feb 2019 19:07:40 +0100 Subject: [PATCH 02/22] remove obsolete fields --- examples/tutorial/x/blog/errors.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/examples/tutorial/x/blog/errors.go b/examples/tutorial/x/blog/errors.go index ba01109c..3b2b79db 100644 --- a/examples/tutorial/x/blog/errors.go +++ b/examples/tutorial/x/blog/errors.go @@ -1,11 +1,5 @@ package blog -import ( - "fmt" - - "github.com/iov-one/weave/errors" -) - // ABCI Response Codes // tutorial reserves 400 ~ 420. const ( @@ -20,15 +14,4 @@ var ( descriptionTooLong = "description is too long" unauthorisedBlogAuthor = "blog author %X" unauthorisedPostAuthor = "post author %X" - - errNegativeArticles = fmt.Errorf("Article count is negative") - errNegativeCreation = fmt.Errorf("Creation block is negative") - - errBlogNotFound = fmt.Errorf("No blog found for post") - errBlogExist = fmt.Errorf("Blog already exists") - errBlogOneAuthorLeft = fmt.Errorf("Unable to remove last blog author") ) - -func ErrBlogOneAuthorLeft() error { - return errors.WithCode(errBlogOneAuthorLeft, CodeInvalidBlog) -} From 1800d55153035a8c5374bf67d11f3fd372922e1f Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 20 Feb 2019 19:07:52 +0100 Subject: [PATCH 03/22] remove unused abci codes --- examples/tutorial/x/blog/errors.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/examples/tutorial/x/blog/errors.go b/examples/tutorial/x/blog/errors.go index 3b2b79db..842967c3 100644 --- a/examples/tutorial/x/blog/errors.go +++ b/examples/tutorial/x/blog/errors.go @@ -1,12 +1,5 @@ package blog -// ABCI Response Codes -// tutorial reserves 400 ~ 420. -const ( - CodeNegativeNumber uint32 = 402 - CodeInvalidBlog uint32 = 403 -) - var ( invalidTitle = "title is too long or too short" invalidText = "text is too long or too short" From fe5a31658aac612c54e47f1edcbeae6bb72f9372 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 13:57:42 +0100 Subject: [PATCH 04/22] review comments --- errors/errors.go | 3 --- examples/tutorial/x/blog/errors.go | 12 ++++++------ examples/tutorial/x/blog/handlers.go | 20 ++++++++++---------- examples/tutorial/x/blog/handlers_test.go | 16 ++++++++-------- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index ce57adb4..3aba6202 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -60,9 +60,6 @@ var ( // ErrInvalidInput stands for general input problems indication ErrInvalidInput = Register(14, "invalid input") - // ErrExists already exists error - ErrExists = Register(15, "already exists") - // ErrPanic is only set when we recover from a panic, so we know to redact potentially sensitive system info ErrPanic = Register(111222, "panic") ) diff --git a/examples/tutorial/x/blog/errors.go b/examples/tutorial/x/blog/errors.go index 842967c3..4da9299a 100644 --- a/examples/tutorial/x/blog/errors.go +++ b/examples/tutorial/x/blog/errors.go @@ -1,10 +1,10 @@ package blog var ( - invalidTitle = "title is too long or too short" - invalidText = "text is too long or too short" - invalidName = "name is too long" - descriptionTooLong = "description is too long" - unauthorisedBlogAuthor = "blog author %X" - unauthorisedPostAuthor = "post author %X" + invalidTitle = "title is too long or too short" + invalidText = "text is too long or too short" + invalidName = "name is too long" + descriptionTooLong = "description is too long" + unauthorisedBlogAuthorFmt = "blog author %X" + unauthorisedPostAuthorFmt = "post author %X" ) diff --git a/examples/tutorial/x/blog/handlers.go b/examples/tutorial/x/blog/handlers.go index 72cacfa3..9f3e4328 100644 --- a/examples/tutorial/x/blog/handlers.go +++ b/examples/tutorial/x/blog/handlers.go @@ -90,7 +90,7 @@ func (h CreateBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil) + return nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, nil) } msg, err := tx.GetMsg() @@ -111,7 +111,7 @@ func (h CreateBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Check the blog does not already exist obj, err := h.bucket.Get(db, []byte(createBlogMsg.Slug)) if err != nil || (obj != nil && obj.Value() != nil) { - return nil, errors.ErrExists.New("blog") + return nil, errors.ErrDuplicate.New("blog") } return createBlogMsg, nil @@ -184,7 +184,7 @@ func (h CreatePostMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Check the author is one of the Tx signer if !h.auth.HasAddress(ctx, createPostMsg.Author) { - return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, createPostMsg.Author) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedPostAuthorFmt, createPostMsg.Author) } err = createPostMsg.Validate() @@ -252,7 +252,7 @@ func (h RenameBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, nil) } msg, err := tx.GetMsg() @@ -283,7 +283,7 @@ func (h RenameBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w blog := obj.Value().(*Blog) // Check main signer is one of the blog authors if findAuthor(blog.Authors, sender.Address()) == -1 { - return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, sender.Address()) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, sender.Address()) } return renameBlogMsg, blog, nil @@ -345,7 +345,7 @@ func (h ChangeBlogAuthorsMsgHandler) validate(ctx weave.Context, db weave.KVStor // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, nil) } msg, err := tx.GetMsg() @@ -375,7 +375,7 @@ func (h ChangeBlogAuthorsMsgHandler) validate(ctx weave.Context, db weave.KVStor blog := obj.Value().(*Blog) // Check main signer is one of the blog authors if findAuthor(blog.Authors, sender.Address()) == -1 { - return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, sender.Address()) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, sender.Address()) } // Get the author index @@ -383,7 +383,7 @@ func (h ChangeBlogAuthorsMsgHandler) validate(ctx weave.Context, db weave.KVStor if changeBlogAuthorsMsg.Add { // When removing an author we must ensure it does not exist already if authorIdx >= 0 { - return nil, nil, errors.ErrExists.Newf("author: %X", changeBlogAuthorsMsg.Author) + return nil, nil, errors.ErrDuplicate.Newf("author: %X", changeBlogAuthorsMsg.Author) } } else { // When removing an author we must ensure : @@ -452,7 +452,7 @@ func (h SetProfileMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, nil) } msg, err := tx.GetMsg() @@ -468,7 +468,7 @@ func (h SetProfileMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w // if author is here we use it for authentication if setProfileMsg.Author != nil { if !h.auth.HasAddress(ctx, setProfileMsg.Author) { - return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, setProfileMsg.Author) + return nil, nil, errors.ErrUnauthorized.Newf(unauthorisedPostAuthorFmt, setProfileMsg.Author) } } diff --git a/examples/tutorial/x/blog/handlers_test.go b/examples/tutorial/x/blog/handlers_test.go index 2929c837..22c388ae 100644 --- a/examples/tutorial/x/blog/handlers_test.go +++ b/examples/tutorial/x/blog/handlers_test.go @@ -257,7 +257,7 @@ func TestCreateBlogMsgHandlerCheck(t *testing.T) { }, { Name: "no signer", - Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil), + Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, nil), Handler: createBlogMsgHandlerFn, Msg: &CreateBlogMsg{ Slug: "this_is_a_blog", @@ -267,7 +267,7 @@ func TestCreateBlogMsgHandlerCheck(t *testing.T) { }, { Name: "creating twice", - Err: errors.ErrExists.New("blog"), + Err: errors.ErrDuplicate.New("blog"), Handler: createBlogMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreateBlogMsg{ @@ -412,7 +412,7 @@ func TestCreatePostMsgHandlerCheck(t *testing.T) { }, { Name: "no author", - Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, nil), + Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthorFmt, nil), Handler: createPostMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreatePostMsg{ @@ -423,7 +423,7 @@ func TestCreatePostMsgHandlerCheck(t *testing.T) { }, { Name: "unauthorized", - Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, unauthorised.Address()), + Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthorFmt, unauthorised.Address()), Handler: createPostMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &CreatePostMsg{ @@ -572,7 +572,7 @@ func TestRenameBlogMsgHandlerCheck(t *testing.T) { }, { Name: "no signer", - Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil), + Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, nil), Handler: renameBlogMsgHandlerFn, Msg: &RenameBlogMsg{ Slug: "this_is_a_blog", @@ -680,7 +680,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "adding existing author", - Err: errors.ErrExists.Newf("author: %X", newAuthor.Address()), + Err: errors.ErrDuplicate.Newf("author: %X", newAuthor.Address()), Handler: changeBlogAuthorsMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &ChangeBlogAuthorsMsg{ @@ -788,7 +788,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "unsigned tx", - Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthor, nil), + Err: errors.ErrUnauthorized.Newf(unauthorisedBlogAuthorFmt, nil), Handler: changeBlogAuthorsMsgHandlerFn, Msg: &ChangeBlogAuthorsMsg{ Slug: "this_is_a_blog", @@ -901,7 +901,7 @@ func TestSetProfileMsgHandlerCheck(t *testing.T) { }, { Name: "unauthorized author", - Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthor, author.Address()), + Err: errors.ErrUnauthorized.Newf(unauthorisedPostAuthorFmt, author.Address()), Handler: SetProfileMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &SetProfileMsg{ From 5198e973f3ed9a575993a31ff1b2f68a4d4dbbe1 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 14:06:00 +0100 Subject: [PATCH 05/22] currency done --- x/currency/errors.go | 31 ------------------------------- x/currency/handler.go | 2 +- x/currency/model.go | 4 ++-- x/currency/msg.go | 5 +++-- 4 files changed, 6 insertions(+), 36 deletions(-) delete mode 100644 x/currency/errors.go diff --git a/x/currency/errors.go b/x/currency/errors.go deleted file mode 100644 index fbf7528f..00000000 --- a/x/currency/errors.go +++ /dev/null @@ -1,31 +0,0 @@ -package currency - -import ( - stderrors "errors" - fmt "fmt" - - "github.com/iov-one/weave/errors" -) - -const ( - CodeInvalidToken = 2000 -) - -var ( - errInvalidTokenName = stderrors.New("Invalid token name") - errInvalidSigFigs = stderrors.New("Invalid significant figures") - errDuplicateToken = stderrors.New("Token with that ticker already exists") -) - -func ErrInvalidSigFigs(figs int32) error { - msg := fmt.Sprintf("%d", figs) - return errors.WithLog(msg, errInvalidSigFigs, CodeInvalidToken) -} - -func ErrInvalidTokenName(name string) error { - return errors.WithLog(name, errInvalidTokenName, CodeInvalidToken) -} - -func ErrDuplicateToken(name string) error { - return errors.WithLog(name, errDuplicateToken, CodeInvalidToken) -} diff --git a/x/currency/handler.go b/x/currency/handler.go index b21bd79a..26af1bd8 100644 --- a/x/currency/handler.go +++ b/x/currency/handler.go @@ -73,7 +73,7 @@ func (h *TokenInfoHandler) validate(ctx weave.Context, db weave.KVStore, tx weav case err != nil: return nil, err case obj != nil: - return nil, ErrDuplicateToken(msg.Ticker) + return nil, errors.ErrDuplicate.Newf("ticker %s", msg.Ticker) } return msg, nil diff --git a/x/currency/model.go b/x/currency/model.go index 5c0c2b49..b284f75b 100644 --- a/x/currency/model.go +++ b/x/currency/model.go @@ -29,10 +29,10 @@ func NewTokenInfo(ticker, name string, sigFigs int32) orm.Object { func (t *TokenInfo) Validate() error { if !isTokenName(t.Name) { - return ErrInvalidTokenName(t.Name) + return errors.ErrInvalidState.Newf("invalid token name %v", t.Name) } if t.SigFigs < minSigFigs || t.SigFigs > maxSigFigs { - return ErrInvalidSigFigs(t.SigFigs) + return errors.ErrInvalidState.Newf("invalid significant figures %d", t.SigFigs) } return nil } diff --git a/x/currency/msg.go b/x/currency/msg.go index 57b5a4ea..585e8f85 100644 --- a/x/currency/msg.go +++ b/x/currency/msg.go @@ -2,6 +2,7 @@ package currency import ( "github.com/iov-one/weave" + "github.com/iov-one/weave/errors" "github.com/iov-one/weave/x" ) @@ -16,10 +17,10 @@ func (t *NewTokenInfoMsg) Validate() error { return x.ErrInvalidCurrency.New(t.Ticker) } if !isTokenName(t.Name) { - return ErrInvalidTokenName(t.Name) + return errors.ErrInvalidState.Newf("invalid token name %v", t.Name) } if t.SigFigs < minSigFigs || t.SigFigs > maxSigFigs { - return ErrInvalidSigFigs(t.SigFigs) + return errors.ErrInvalidState.Newf("invalid significant figures %d", t.SigFigs) } return nil } From 4c64d0b3f8442466642690d2f0d8c1c308b67d93 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 14:18:41 +0100 Subject: [PATCH 06/22] more refactoring and legacy test removal --- cmd/bnsd/x/nft/username/handler.go | 8 +-- errors/common.go | 22 --------- errors/common_test.go | 79 +----------------------------- x/cash/decorator.go | 4 +- x/cash/handler.go | 4 +- x/currency/handler.go | 2 +- x/currency/handler_test.go | 16 +++--- x/escrow/errors.go | 22 --------- x/escrow/handler.go | 10 ++-- x/escrow/model.go | 6 +-- x/escrow/msg.go | 6 +-- x/escrow/msg_test.go | 4 +- x/multisig/handlers.go | 2 +- x/namecoin/handler.go | 4 +- x/nft/base/handler.go | 2 +- 15 files changed, 35 insertions(+), 156 deletions(-) diff --git a/cmd/bnsd/x/nft/username/handler.go b/cmd/bnsd/x/nft/username/handler.go index 95dd4c6b..51a0319c 100644 --- a/cmd/bnsd/x/nft/username/handler.go +++ b/cmd/bnsd/x/nft/username/handler.go @@ -82,11 +82,11 @@ func (h IssueHandler) validate(ctx weave.Context, tx weave.Tx) (*IssueTokenMsg, // check permissions if h.issuer != nil { if !h.auth.HasAddress(ctx, h.issuer) { - return nil, errors.ErrUnauthorizedLegacy() + return nil, errors.ErrUnauthorized } } else { if !h.auth.HasAddress(ctx, msg.Owner) { - return nil, errors.ErrUnauthorizedLegacy() + return nil, errors.ErrUnauthorized } } return msg, nil @@ -122,7 +122,7 @@ func (h AddChainAddressHandler) Deliver(ctx weave.Context, store weave.KVStore, actor := nft.FindActor(h.auth, ctx, t, nft.UpdateDetails) if actor == nil { - return res, errors.ErrUnauthorizedLegacy() + return res, errors.ErrUnauthorized } allKeys := append(t.GetChainAddresses(), ChainAddress{msg.GetBlockchainID(), msg.GetAddress()}) if err := t.SetChainAddresses(actor, allKeys); err != nil { @@ -177,7 +177,7 @@ func (h RemoveChainAddressHandler) Deliver(ctx weave.Context, store weave.KVStor actor := nft.FindActor(h.auth, ctx, t, nft.UpdateDetails) if actor == nil { - return res, errors.ErrUnauthorizedLegacy() + return res, errors.ErrUnauthorized } if len(t.GetChainAddresses()) == 0 { return res, nft.ErrInvalidEntry([]byte("no chain to delete")) diff --git a/errors/common.go b/errors/common.go index 5e4c2eb1..1cc8ffec 100644 --- a/errors/common.go +++ b/errors/common.go @@ -136,10 +136,6 @@ func ErrDecoding() error { return WithCode(errDecoding, CodeTxParseError) } -// IsDecodingErr returns true for any error with a ParseError code -func IsDecodingErr(err error) bool { - return HasErrorCode(err, CodeTxParseError) -} // ErrTooLarge is a specific decode error when we pass the max tx size func ErrTooLarge() error { @@ -152,12 +148,6 @@ func IsTooLargeErr(err error) bool { return IsSameError(errTooLarge, err) } -// ErrUnauthorizedLegacy is a generic denial. -// You can use a more specific cause if you wish, such as ErrInvalidSignature -func ErrUnauthorizedLegacy() error { - return WithCode(errUnauthorized, CodeUnauthorized) -} - // IsUnauthorizedErr is generic helper for any unauthorized errors, // also specific sub-types func IsUnauthorizedErr(err error) bool { @@ -192,24 +182,12 @@ func ErrInvalidChainID(chainID string) error { return WithLog(chainID, errInvalidChainID, CodeInvalidChainID) } -// IsInvalidChainIDErr returns true iff an error was created -// with ErrInvalidChainID -func IsInvalidChainIDErr(err error) bool { - return IsSameError(errInvalidChainID, err) -} - // ErrModifyChainID is when someone tries to change the chainID // after genesis func ErrModifyChainID() error { return WithCode(errModifyChainID, CodeInvalidChainID) } -// IsModifyChainIDErr returns true iff an error was created -// with ErrModifyChainID -func IsModifyChainIDErr(err error) bool { - return IsSameError(errModifyChainID, err) -} - func WithType(err error, obj interface{}) error { return Wrap(err, fmt.Sprintf("%T", obj)) } diff --git a/errors/common_test.go b/errors/common_test.go index a7526896..1d705a51 100644 --- a/errors/common_test.go +++ b/errors/common_test.go @@ -2,7 +2,6 @@ package errors import ( "fmt" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -23,26 +22,9 @@ func TestChecks(t *testing.T) { check CheckErr match bool }{ - // specific errors match broader checks, but not visa versa - {ErrDecoding(), IsDecodingErr, true}, - {ErrTooLarge(), IsTooLargeErr, true}, - {ErrTooLarge(), IsDecodingErr, true}, - {ErrDecoding(), IsTooLargeErr, false}, - {ErrUnauthorizedLegacy(), IsDecodingErr, false}, - {ErrUnauthorizedLegacy(), IsUnauthorizedErr, true}, // make sure lots of things match ErrInternal, but not everything - {ErrInternalLegacy("bad db connection"), IsInternalErr, true}, {Wrap(fmt.Errorf("wrapped"), "wrapped"), IsInternalErr, true}, - {fmt.Errorf("wrapped"), IsInternalErr, true}, - {ErrUnauthorizedLegacy(), IsInternalErr, false}, - - {ErrMissingSignature(), IsUnauthorizedErr, true}, - {ErrMissingSignature(), IsMissingSignatureErr, true}, - {ErrUnauthorizedLegacy(), IsMissingSignatureErr, false}, - {ErrInvalidSignature(), IsUnauthorizedErr, true}, - {ErrInvalidSignature(), IsInvalidSignatureErr, true}, - {nil, NoErr, true}, {Wrap(nil, "asd"), NoErr, true}, } @@ -51,63 +33,4 @@ func TestChecks(t *testing.T) { match := tc.check(tc.err) assert.Equal(t, tc.match, match, "%d", i) } -} - -// TestLog checks the text returned by the error -func TestLog(t *testing.T) { - cases := []struct { - err error - // this should always pass, just to verify - check CheckErr - // this is the text we want to see with .Log() - log string - }{ - // make sure messages are nice, even if wrapped or not - {ErrTooLarge(), IsTooLargeErr, "(2) Input size too large"}, - {Wrap(ErrTooLarge(), ""), IsTooLargeErr, ": Input size too large"}, - {Wrap(fmt.Errorf("wrapped"), ""), IsInternalErr, ": wrapped"}, - - // with code shouldn't change the error message - {WithCode(ErrUnauthorizedLegacy(), CodeTxParseError), IsDecodingErr, "(2) Unauthorized"}, - - // with log should add some in front - {WithLog("Special", ErrUnauthorizedLegacy(), CodeInternalErr), IsInternalErr, "(1) Special: Unauthorized"}, - - // verify some standard message types with prefixes - {ErrUnrecognizedAddress([]byte{0, 0x12, 0x77}), IsUnrecognizedAddressErr, "(5) 001277: Unrecognized Address"}, - {ErrUnrecognizedCondition([]byte{0xF0, 0x0D, 0xCA, 0xFE}), IsUnrecognizedConditionErr, "(5) F00DCAFE: Unrecognized Condition"}, - {ErrUnknownTxType("john_123"), IsUnknownTxTypeErr, "(4) string: Tx type unknown"}, - {ErrUnknownTxType(t), IsUnknownTxTypeErr, "(4) *testing.T: Tx type unknown"}, - } - - for i, tc := range cases { - t.Run(fmt.Sprintf("case-%d", i), func(t *testing.T) { - - assert.True(t, tc.check(tc.err)) - - // make sure we have a nice error message with code - msg := fmt.Sprintf("%s", tc.err) - assert.Equal(t, tc.log, msg) - - // make sure we have a nice error message with code - middle := fmt.Sprintf("%v", tc.err) - assert.Contains(t, middle, tc.log) - - // TODO: this is failing, because stacktrace - // implementation is not present for the new error - // handing code. - // assert.Contains(t, middle, "common_test.go", middle) - - // make sure we also get stack dumps.... - stack := fmt.Sprintf("%+v", tc.err) - // we should trim off unneeded stuff - withCode := "github.com/iov-one/weave/errors.WithCode\n" - // thisTest := "github.com/iov-one/weave/errors.TestLog\n" - assert.False(t, strings.Contains(stack, withCode)) - // TODO: this is failing, because stacktrace - // implementation is not present for the new error - // handing code. - // assert.True(t, strings.Contains(stack, thisTest)) - }) - } -} +} \ No newline at end of file diff --git a/x/cash/decorator.go b/x/cash/decorator.go index 71f800aa..2022591c 100644 --- a/x/cash/decorator.go +++ b/x/cash/decorator.go @@ -63,7 +63,7 @@ func (d FeeDecorator) Check(ctx weave.Context, store weave.KVStore, tx weave.Tx, // verify we have access to the money if !d.auth.HasAddress(ctx, finfo.Payer) { - return res, errors.ErrUnauthorizedLegacy() + return res, errors.ErrUnauthorized } // and have enough collector := gconf.Address(store, GconfCollectorAddress) @@ -97,7 +97,7 @@ func (d FeeDecorator) Deliver(ctx weave.Context, store weave.KVStore, tx weave.T // verify we have access to the money if !d.auth.HasAddress(ctx, finfo.Payer) { - return res, errors.ErrUnauthorizedLegacy() + return res, errors.ErrUnauthorized } // and subtract it from the account collector := gconf.Address(store, GconfCollectorAddress) diff --git a/x/cash/handler.go b/x/cash/handler.go index d253c57a..5506a04d 100644 --- a/x/cash/handler.go +++ b/x/cash/handler.go @@ -58,7 +58,7 @@ func (h SendHandler) Check(ctx weave.Context, store weave.KVStore, // make sure we have permission from the sender if !h.auth.HasAddress(ctx, msg.Src) { - return res, errors.ErrUnauthorizedLegacy() + return res, errors.ErrUnauthorized } // return cost @@ -89,7 +89,7 @@ func (h SendHandler) Deliver(ctx weave.Context, store weave.KVStore, // make sure we have permission from the sender if !h.auth.HasAddress(ctx, msg.Src) { - return res, errors.ErrUnauthorizedLegacy() + return res, errors.ErrUnauthorized } // move the money.... diff --git a/x/currency/handler.go b/x/currency/handler.go index 26af1bd8..19225c3b 100644 --- a/x/currency/handler.go +++ b/x/currency/handler.go @@ -65,7 +65,7 @@ func (h *TokenInfoHandler) validate(ctx weave.Context, db weave.KVStore, tx weav // Ensure we have permission if the issuer is provided. if h.issuer != nil && !h.auth.HasAddress(ctx, h.issuer) { - return nil, errors.ErrUnauthorizedLegacy() + return nil, errors.ErrUnauthorized } // Token can be registered only once and must not be updated. diff --git a/x/currency/handler_test.go b/x/currency/handler_test.go index 8943e38d..8cb68e7d 100644 --- a/x/currency/handler_test.go +++ b/x/currency/handler_test.go @@ -22,8 +22,8 @@ func TestNewTokenInfoHandler(t *testing.T) { issuer weave.Address initState []orm.Object msg weave.Msg - wantCheckErr uint32 - wantDeliverErr uint32 + wantCheckErr errors.Error + wantDeliverErr errors.Error query string wantQueryResult orm.Object }{ @@ -34,15 +34,15 @@ func TestNewTokenInfoHandler(t *testing.T) { orm.NewSimpleObj([]byte("DOGE"), &TokenInfo{Name: "Doge Coin", SigFigs: 6}), }, msg: &NewTokenInfoMsg{Ticker: "DOGE", Name: "Doge Coin", SigFigs: 6}, - wantCheckErr: CodeInvalidToken, - wantDeliverErr: CodeInvalidToken, + wantCheckErr: errors.ErrDuplicate, + wantDeliverErr: errors.ErrDuplicate, }, "insufficient permission": { signers: []weave.Condition{permB}, issuer: permA.Address(), msg: &NewTokenInfoMsg{Ticker: "DOGE", Name: "Doge Coin", SigFigs: 6}, - wantCheckErr: errors.CodeUnauthorized, - wantDeliverErr: errors.CodeUnauthorized, + wantCheckErr: errors.ErrUnauthorized, + wantDeliverErr: errors.ErrUnauthorized, }, "query unknown ticker": { signers: []weave.Condition{permA, permB}, @@ -73,10 +73,10 @@ func TestNewTokenInfoHandler(t *testing.T) { auth := helpers.Authenticate(tc.signers...) h := NewTokenInfoHandler(auth, tc.issuer) tx := helpers.MockTx(tc.msg) - if _, err := h.Check(nil, db, tx); errcode(err) != tc.wantCheckErr { + if _, err := h.Check(nil, db, tx); tc.wantCheckErr.Is(err) { t.Fatalf("check error: want %d, got %+v", tc.wantCheckErr, err) } - if _, err := h.Deliver(nil, db, tx); errcode(err) != tc.wantDeliverErr { + if _, err := h.Deliver(nil, db, tx); tc.wantCheckErr.Is(err) { t.Fatalf("deliver error: want %d, got %+v", tc.wantCheckErr, err) } diff --git a/x/escrow/errors.go b/x/escrow/errors.go index 2d59983f..32a2ef65 100644 --- a/x/escrow/errors.go +++ b/x/escrow/errors.go @@ -10,8 +10,6 @@ import ( // escrow takes 1010-1020 const ( CodeNoEscrow = 1010 - CodeMissingCondition = 1011 - CodeInvalidCondition = 1012 CodeInvalidMetadata = 1013 CodeInvalidHeight = 1014 @@ -20,10 +18,6 @@ const ( ) var ( - errMissingArbiter = fmt.Errorf("Missing Arbiter") - errMissingSender = fmt.Errorf("Missing Src") - errMissingRecipient = fmt.Errorf("Missing Recipient") - errMissingAllConditions = fmt.Errorf("Missing All Conditions") errInvalidMemo = fmt.Errorf("Memo field too long") errInvalidTimeout = fmt.Errorf("Invalid Timeout") @@ -40,22 +34,6 @@ var ( // errNoSuchWallet = fmt.Errorf("No wallet exists with this address") ) -func ErrMissingArbiter() error { - return errors.WithCode(errMissingArbiter, CodeMissingCondition) -} -func ErrMissingSender() error { - return errors.WithCode(errMissingSender, CodeMissingCondition) -} -func ErrMissingRecipient() error { - return errors.WithCode(errMissingRecipient, CodeMissingCondition) -} -func ErrMissingAllConditions() error { - return errors.WithCode(errMissingAllConditions, CodeMissingCondition) -} -func IsMissingConditionErr(err error) bool { - return errors.HasErrorCode(err, CodeMissingCondition) -} - func ErrInvalidCondition(perm []byte) error { return errors.ErrUnrecognizedCondition(perm) } diff --git a/x/escrow/handler.go b/x/escrow/handler.go index 07b9fb1f..e86c5229 100644 --- a/x/escrow/handler.go +++ b/x/escrow/handler.go @@ -125,7 +125,7 @@ func (h CreateEscrowHandler) validate(ctx weave.Context, db weave.KVStore, if msg.Src != nil { sender := weave.Address(msg.Src) if !h.auth.HasAddress(ctx, sender) { - return nil, errors.ErrUnauthorizedLegacy() + return nil, errors.ErrUnauthorized } } @@ -222,7 +222,7 @@ func (h ReleaseEscrowHandler) validate(ctx weave.Context, db weave.KVStore, arb := weave.Condition(escrow.Arbiter).Address() sender := weave.Address(escrow.Sender) if !h.auth.HasAddress(ctx, arb) && !h.auth.HasAddress(ctx, sender) { - return nil, nil, errors.ErrUnauthorizedLegacy() + return nil, nil, errors.ErrUnauthorized } // timeout must not have expired @@ -400,19 +400,19 @@ func (h UpdateEscrowHandler) validate(ctx weave.Context, db weave.KVStore, if msg.Sender != nil { sender := weave.Address(escrow.Sender) if !h.auth.HasAddress(ctx, sender) { - return nil, nil, errors.ErrUnauthorizedLegacy() + return nil, nil, errors.ErrUnauthorized } } if msg.Recipient != nil { rcpt := weave.Address(escrow.Recipient) if !h.auth.HasAddress(ctx, rcpt) { - return nil, nil, errors.ErrUnauthorizedLegacy() + return nil, nil, errors.ErrUnauthorized } } if msg.Arbiter != nil { arbiter := weave.Condition(escrow.Arbiter).Address() if !h.auth.HasAddress(ctx, arbiter) { - return nil, nil, errors.ErrUnauthorizedLegacy() + return nil, nil, errors.ErrUnauthorized } } diff --git a/x/escrow/model.go b/x/escrow/model.go index 90487a58..8c96df5a 100644 --- a/x/escrow/model.go +++ b/x/escrow/model.go @@ -19,15 +19,15 @@ var _ orm.CloneableData = (*Escrow)(nil) // Validate ensures the escrow is valid func (e *Escrow) Validate() error { if e.Sender == nil { - return ErrMissingSender() + return errors.ErrEmpty.New("sender") } // Copied from CreateEscrowMsg.Validate // TODO: code reuse??? if e.Arbiter == nil { - return ErrMissingArbiter() + return errors.ErrEmpty.New("arbiter") } if e.Recipient == nil { - return ErrMissingRecipient() + return errors.ErrEmpty.New("recipient") } if e.Timeout <= 0 { return ErrInvalidTimeout(e.Timeout) diff --git a/x/escrow/msg.go b/x/escrow/msg.go index 939ad2d1..f3aa59a1 100644 --- a/x/escrow/msg.go +++ b/x/escrow/msg.go @@ -60,10 +60,10 @@ func NewCreateMsg(send, rcpt weave.Address, arb weave.Condition, // Validate makes sure that this is sensible func (m *CreateEscrowMsg) Validate() error { if m.Arbiter == nil { - return ErrMissingArbiter() + return errors.ErrEmpty.New("arbiter") } if m.Recipient == nil { - return ErrMissingRecipient() + return errors.ErrEmpty.New("recipient") } if m.Timeout <= 0 { return ErrInvalidTimeout(m.Timeout) @@ -107,7 +107,7 @@ func (m *UpdateEscrowPartiesMsg) Validate() error { if m.Arbiter == nil && m.Sender == nil && m.Recipient == nil { - return ErrMissingAllConditions() + return errors.ErrEmpty.New("all conditions") } err = validateConditions(m.Arbiter) if err != nil { diff --git a/x/escrow/msg_test.go b/x/escrow/msg_test.go index b5707558..452589d9 100644 --- a/x/escrow/msg_test.go +++ b/x/escrow/msg_test.go @@ -46,7 +46,7 @@ func TestCreateEscrowMsg(t *testing.T) { check checkErr }{ // nothing - 0: {new(CreateEscrowMsg), IsMissingConditionErr}, + 0: {new(CreateEscrowMsg), errors.ErrEmpty.Is}, // proper 1: { &CreateEscrowMsg{ @@ -286,7 +286,7 @@ func TestUpdateEscrowMsg(t *testing.T) { &UpdateEscrowPartiesMsg{ EscrowId: escrow, }, - IsMissingConditionErr, + errors.ErrEmpty.Is, }, // invalid escrow, proper permissions 3: { diff --git a/x/multisig/handlers.go b/x/multisig/handlers.go index 424aa01d..bb571c23 100644 --- a/x/multisig/handlers.go +++ b/x/multisig/handlers.go @@ -64,7 +64,7 @@ func (h CreateContractMsgHandler) validate(ctx weave.Context, db weave.KVStore, // Retrieve tx main signer in this context sender := x.MainSigner(ctx, h.auth) if sender == nil { - return nil, errors.ErrUnauthorizedLegacy() + return nil, errors.ErrUnauthorized } msg, err := tx.GetMsg() diff --git a/x/namecoin/handler.go b/x/namecoin/handler.go index eff7764d..e3ef2307 100644 --- a/x/namecoin/handler.go +++ b/x/namecoin/handler.go @@ -116,7 +116,7 @@ func (h TokenHandler) validate(ctx weave.Context, db weave.KVStore, // make sure we have permission if the issuer is set if h.issuer != nil && !h.auth.HasAddress(ctx, h.issuer) { - return nil, errors.ErrUnauthorizedLegacy() + return nil, errors.ErrUnauthorized } // make sure no token there yet @@ -202,7 +202,7 @@ func (h SetNameHandler) validate(ctx weave.Context, db weave.KVStore, // only wallet owner can set the name if !h.auth.HasAddress(ctx, msg.Address) { - return nil, errors.ErrUnauthorizedLegacy() + return nil, errors.ErrUnauthorized } return msg, nil diff --git a/x/nft/base/handler.go b/x/nft/base/handler.go index d3039053..b15d9b35 100644 --- a/x/nft/base/handler.go +++ b/x/nft/base/handler.go @@ -94,7 +94,7 @@ func (h *ApprovalOpsHandler) Deliver(ctx weave.Context, store weave.KVStore, tx actor := nft.FindActor(h.auth, ctx, t, nft.UpdateApprovals) if actor == nil { - return res, errors.ErrUnauthorizedLegacy() + return res, errors.ErrUnauthorized } switch v := msg.(type) { From 8a3675254c2958b6e03ceb7a75a1b87f86939478 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 14:44:19 +0100 Subject: [PATCH 07/22] fix currency tests --- x/currency/handler_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/currency/handler_test.go b/x/currency/handler_test.go index 8cb68e7d..58f26525 100644 --- a/x/currency/handler_test.go +++ b/x/currency/handler_test.go @@ -74,10 +74,10 @@ func TestNewTokenInfoHandler(t *testing.T) { h := NewTokenInfoHandler(auth, tc.issuer) tx := helpers.MockTx(tc.msg) if _, err := h.Check(nil, db, tx); tc.wantCheckErr.Is(err) { - t.Fatalf("check error: want %d, got %+v", tc.wantCheckErr, err) + t.Fatalf("check error: want %v, got %+v", tc.wantCheckErr, err) } if _, err := h.Deliver(nil, db, tx); tc.wantCheckErr.Is(err) { - t.Fatalf("deliver error: want %d, got %+v", tc.wantCheckErr, err) + t.Fatalf("deliver error: want %v, got %+v", tc.wantCheckErr, err) } if res, err := bucket.Get(db, tc.query); err != nil { From 239d2bb4c8aa730deb242d5c1fb1f159228b6505 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 15:08:34 +0100 Subject: [PATCH 08/22] fix tests --- x/cash/decorator_test.go | 2 +- x/cash/handler_test.go | 4 ++-- x/currency/handler_test.go | 14 ++++++++++---- x/multisig/decorator.go | 3 ++- x/multisig/decorator_test.go | 5 +++-- x/multisig/handlers.go | 2 +- x/multisig/handlers_test.go | 7 ++++--- x/namecoin/handler_test.go | 12 ++++++------ 8 files changed, 29 insertions(+), 20 deletions(-) diff --git a/x/cash/decorator_test.go b/x/cash/decorator_test.go index 40fa04ef..a7bb6eaa 100644 --- a/x/cash/decorator_test.go +++ b/x/cash/decorator_test.go @@ -110,7 +110,7 @@ func TestFees(t *testing.T) { []orm.Object{must(WalletWith(perm2.Address(), &cash))}, &FeeInfo{Payer: perm2.Address(), Fees: &min}, min, - errors.IsUnauthorizedErr, + errors.ErrUnauthorized.Is, }, // can pay in any fee 7: { diff --git a/x/cash/handler_test.go b/x/cash/handler_test.go index 79948fe1..cfea1922 100644 --- a/x/cash/handler_test.go +++ b/x/cash/handler_test.go @@ -40,8 +40,8 @@ func TestSend(t *testing.T) { nil, nil, &SendMsg{Amount: &foo, Src: perm.Address(), Dest: perm2.Address()}, - errors.IsUnauthorizedErr, - errors.IsUnauthorizedErr, + errors.ErrUnauthorized.Is, + errors.ErrUnauthorized.Is, }, // sender has no account 4: { diff --git a/x/currency/handler_test.go b/x/currency/handler_test.go index 58f26525..282bbc2a 100644 --- a/x/currency/handler_test.go +++ b/x/currency/handler_test.go @@ -73,11 +73,17 @@ func TestNewTokenInfoHandler(t *testing.T) { auth := helpers.Authenticate(tc.signers...) h := NewTokenInfoHandler(auth, tc.issuer) tx := helpers.MockTx(tc.msg) - if _, err := h.Check(nil, db, tx); tc.wantCheckErr.Is(err) { - t.Fatalf("check error: want %v, got %+v", tc.wantCheckErr, err) + _, err := h.Check(nil, db, tx) + if err != nil { + if !tc.wantCheckErr.Is(err) { + t.Fatalf("check error: want %v, got %+v", tc.wantCheckErr, err) + } } - if _, err := h.Deliver(nil, db, tx); tc.wantCheckErr.Is(err) { - t.Fatalf("deliver error: want %v, got %+v", tc.wantCheckErr, err) + _, err = h.Deliver(nil, db, tx) + if err != nil { + if !tc.wantDeliverErr.Is(err) { + t.Fatalf("deliver error: want %v, got %+v", tc.wantDeliverErr, err) + } } if res, err := bucket.Get(db, tc.query); err != nil { diff --git a/x/multisig/decorator.go b/x/multisig/decorator.go index f55449d5..ce5b8b37 100644 --- a/x/multisig/decorator.go +++ b/x/multisig/decorator.go @@ -2,6 +2,7 @@ package multisig import ( "github.com/iov-one/weave" + "github.com/iov-one/weave/errors" "github.com/iov-one/weave/x" ) @@ -68,7 +69,7 @@ func (d Decorator) withMultisig(ctx weave.Context, store weave.KVStore, tx weave // check sigs (can be sig or multisig) authenticated := x.HasNAddresses(ctx, d.auth, sigs, int(contract.ActivationThreshold)) if !authenticated { - return ctx, ErrUnauthorizedMultiSig(contractID) + return ctx, errors.ErrUnauthorized.Newf("contract=%X", contractID) } ctx = withMultisig(ctx, contractID) diff --git a/x/multisig/decorator_test.go b/x/multisig/decorator_test.go index 70797e68..7292504b 100644 --- a/x/multisig/decorator_test.go +++ b/x/multisig/decorator_test.go @@ -2,6 +2,7 @@ package multisig import ( "fmt" + "github.com/iov-one/weave/errors" "testing" "github.com/stretchr/testify/assert" @@ -83,7 +84,7 @@ func TestDecorator(t *testing.T) { multisigTx([]byte("foo"), contractID1), []weave.Condition{a}, nil, - ErrUnauthorizedMultiSig(contractID1), + errors.ErrUnauthorized.Newf("contract=%X", contractID1), }, // with invalid multisig contract ID { @@ -111,7 +112,7 @@ func TestDecorator(t *testing.T) { multisigTx([]byte("foo"), contractID3), []weave.Condition{d, e}, // cconditions for ontractID2 are there but ontractID2 must be passed explicitly nil, - ErrUnauthorizedMultiSig(contractID3), + errors.ErrUnauthorized.Newf("contract=%X", contractID3), }, } diff --git a/x/multisig/handlers.go b/x/multisig/handlers.go index bb571c23..4f933952 100644 --- a/x/multisig/handlers.go +++ b/x/multisig/handlers.go @@ -161,7 +161,7 @@ func (h UpdateContractMsgHandler) validate(ctx weave.Context, db weave.KVStore, // check sigs authenticated := x.HasNAddresses(ctx, h.auth, sigs, int(contract.AdminThreshold)) if !authenticated { - return nil, ErrUnauthorizedMultiSig(updateContractMsg.Id) + return nil, errors.ErrUnauthorized.Newf("contract=%X", updateContractMsg.Id) } return updateContractMsg, nil diff --git a/x/multisig/handlers_test.go b/x/multisig/handlers_test.go index 9da997a9..dc382cbc 100644 --- a/x/multisig/handlers_test.go +++ b/x/multisig/handlers_test.go @@ -2,6 +2,7 @@ package multisig import ( "context" + "github.com/iov-one/weave/errors" "testing" "github.com/stretchr/testify/require" @@ -194,7 +195,7 @@ func TestUpdateContractMsgHandler(t *testing.T) { AdminThreshold: 5, }, signers: []weave.Condition{a}, - err: ErrUnauthorizedMultiSig(mutableID), + err: errors.ErrUnauthorized.Newf("contract=%X", mutableID), }, { name: "immutable", @@ -205,7 +206,7 @@ func TestUpdateContractMsgHandler(t *testing.T) { AdminThreshold: 5, }, signers: []weave.Condition{a, b, c, d, e}, - err: ErrUnauthorizedMultiSig(immutableID), + err: errors.ErrUnauthorized.Newf("contract=%X", immutableID), }, { name: "bad change threshold", @@ -229,7 +230,7 @@ func TestUpdateContractMsgHandler(t *testing.T) { if test.err == nil { require.NoError(t, err, test.name) } else { - require.EqualError(t, err, test.err.Error(), test.name) + require.True(t, errors.Is(err, test.err), test.name) } _, err = handler.Deliver(ctx, db, newTx(msg)) diff --git a/x/namecoin/handler_test.go b/x/namecoin/handler_test.go index ce223fd6..c53f7873 100644 --- a/x/namecoin/handler_test.go +++ b/x/namecoin/handler_test.go @@ -53,8 +53,8 @@ func TestSendHandler(t *testing.T) { nil, nil, &cash.SendMsg{Amount: &foo, Src: addr, Dest: addr2}, - errors.IsUnauthorizedErr, - errors.IsUnauthorizedErr, + errors.ErrUnauthorized.Is, + errors.ErrUnauthorized.Is, }, // sender has no account 4: { @@ -151,9 +151,9 @@ func TestNewTokenHandler(t *testing.T) { noErr, noErr, ticker, added}, // not enough permissions 7: {nil, addr, nil, msg, - errors.IsUnauthorizedErr, errors.IsUnauthorizedErr, "", nil}, + errors.ErrUnauthorized.Is, errors.ErrUnauthorized.Is, "", nil}, 8: {[]weave.Condition{perm2, perm3}, addr, nil, msg, - errors.IsUnauthorizedErr, errors.IsUnauthorizedErr, "", nil}, + errors.ErrUnauthorized.Is, errors.ErrUnauthorized.Is, "", nil}, // now have permission 9: {[]weave.Condition{perm2, perm3}, addr2, nil, msg, noErr, noErr, ticker, added}, @@ -226,12 +226,12 @@ func TestSetNameHandler(t *testing.T) { IsInvalidWallet, IsInvalidWallet, nil, nil}, // no permission to change account 3: {nil, []orm.Object{newUser}, msg, - errors.IsUnauthorizedErr, errors.IsUnauthorizedErr, nil, nil}, + errors.ErrUnauthorized.Is, errors.ErrUnauthorized.Is, nil, nil}, // no account to change - only checked deliver 4: {perm, nil, msg, noErr, IsInvalidWallet, nil, nil}, 5: {perm2, []orm.Object{newUser}, msg, - errors.IsUnauthorizedErr, errors.IsUnauthorizedErr, nil, nil}, + errors.ErrUnauthorized.Is, errors.ErrUnauthorized.Is, nil, nil}, // yes, we changed it! 6: {perm, []orm.Object{newUser}, msg, noErr, noErr, addr, setUser}, From c4a3aabc823b99b572b9e8994e792ba25fad5389 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 15:24:13 +0100 Subject: [PATCH 09/22] fix tests --- cmd/bnsd/x/nft/username/model.go | 2 +- cmd/bnsd/x/nft/username/msg.go | 2 +- errors/common.go | 13 ------------- errors/common_test.go | 5 +++-- examples/errors/demo.go | 2 +- x/cash/decorator_test.go | 4 ++-- x/escrow/errors.go | 7 +++---- x/multisig/handlers_test.go | 2 +- x/nft/approvals.go | 6 +++--- x/nft/msg.go | 8 ++++---- 10 files changed, 19 insertions(+), 32 deletions(-) diff --git a/cmd/bnsd/x/nft/username/model.go b/cmd/bnsd/x/nft/username/model.go index 9c9b11a6..310e85c5 100644 --- a/cmd/bnsd/x/nft/username/model.go +++ b/cmd/bnsd/x/nft/username/model.go @@ -81,7 +81,7 @@ func (t *TokenDetails) Clone() *TokenDetails { func (t *TokenDetails) Validate() error { if t == nil { - return errors.ErrInternalLegacy("must not be nil") + return errors.ErrInternal.New("must not be nil") } dup := containsDuplicateChains(t.Addresses) if dup != nil { diff --git a/cmd/bnsd/x/nft/username/msg.go b/cmd/bnsd/x/nft/username/msg.go index 4228d1ec..f9baf557 100644 --- a/cmd/bnsd/x/nft/username/msg.go +++ b/cmd/bnsd/x/nft/username/msg.go @@ -78,7 +78,7 @@ func (m *RemoveChainAddressMsg) Validate() error { func validateID(id []byte) error { if id == nil { - return errors.ErrInternalLegacy("must not be nil") + return errors.ErrInternal.New("must not be nil") } if !isValidID(string(id)) { return nft.ErrInvalidID(id) diff --git a/errors/common.go b/errors/common.go index 1cc8ffec..fa95d360 100644 --- a/errors/common.go +++ b/errors/common.go @@ -23,7 +23,6 @@ var ( errDecoding = stderrors.New("Error decoding input") errTooLarge = stderrors.New("Input size too large") errUnknownTxType = stderrors.New("Tx type unknown") - errUnauthorized = stderrors.New("Unauthorized") errMissingSignature = stderrors.New("Signature missing") errInvalidSignature = stderrors.New("Signature invalid") errUnrecognizedAddress = stderrors.New("Unrecognized Address") @@ -120,23 +119,11 @@ func IsUnrecognizedConditionErr(err error) bool { return IsSameError(errUnrecognizedCondition, err) } -// ErrInternalLegacy is a generic error code when we cannot return any more -// useful info -func ErrInternalLegacy(msg string) error { - return New(msg, CodeInternalErr) -} - -// IsInternalErr returns true for any error that is not classified -func IsInternalErr(err error) bool { - return HasErrorCode(err, CodeInternalErr) -} - // ErrDecoding is generic when we cannot parse the transaction input func ErrDecoding() error { return WithCode(errDecoding, CodeTxParseError) } - // ErrTooLarge is a specific decode error when we pass the max tx size func ErrTooLarge() error { return WithCode(errTooLarge, CodeTxParseError) diff --git a/errors/common_test.go b/errors/common_test.go index 1d705a51..97522153 100644 --- a/errors/common_test.go +++ b/errors/common_test.go @@ -24,7 +24,8 @@ func TestChecks(t *testing.T) { }{ // make sure lots of things match ErrInternal, but not everything - {Wrap(fmt.Errorf("wrapped"), "wrapped"), IsInternalErr, true}, + {Wrap(fmt.Errorf("internal"), "wrapped"), + func(err error) bool { return !Is(err, ErrInternal.New("wrapped")) }, true}, {nil, NoErr, true}, {Wrap(nil, "asd"), NoErr, true}, } @@ -33,4 +34,4 @@ func TestChecks(t *testing.T) { match := tc.check(tc.err) assert.Equal(t, tc.match, match, "%d", i) } -} \ No newline at end of file +} diff --git a/examples/errors/demo.go b/examples/errors/demo.go index 8d81bfbc..0a49746c 100644 --- a/examples/errors/demo.go +++ b/examples/errors/demo.go @@ -12,7 +12,7 @@ import ( ) func makeError() error { - return errors.ErrInternalLegacy("foo") + return errors.ErrInternal.New("foo") } func otherError() error { diff --git a/x/cash/decorator_test.go b/x/cash/decorator_test.go index a7bb6eaa..d41c03a3 100644 --- a/x/cash/decorator_test.go +++ b/x/cash/decorator_test.go @@ -30,11 +30,11 @@ func (f feeTx) GetFees() *FeeInfo { } func (f feeTx) Marshal() ([]byte, error) { - return nil, errors.ErrInternalLegacy("TODO: not implemented") + return nil, errors.ErrInternal.New("TODO: not implemented") } func (f *feeTx) Unmarshal([]byte) error { - return errors.ErrInternalLegacy("TODO: not implemented") + return errors.ErrInternal.New("TODO: not implemented") } type okHandler struct{} diff --git a/x/escrow/errors.go b/x/escrow/errors.go index 32a2ef65..6a1c120e 100644 --- a/x/escrow/errors.go +++ b/x/escrow/errors.go @@ -9,16 +9,15 @@ import ( // ABCI Response Codes // escrow takes 1010-1020 const ( - CodeNoEscrow = 1010 - CodeInvalidMetadata = 1013 - CodeInvalidHeight = 1014 + CodeNoEscrow = 1010 + CodeInvalidMetadata = 1013 + CodeInvalidHeight = 1014 // CodeInvalidIndex = 1001 // CodeInvalidWallet = 1002 ) var ( - errInvalidMemo = fmt.Errorf("Memo field too long") errInvalidTimeout = fmt.Errorf("Invalid Timeout") errInvalidEscrowID = fmt.Errorf("Invalid Escrow ID") diff --git a/x/multisig/handlers_test.go b/x/multisig/handlers_test.go index dc382cbc..932d1ebe 100644 --- a/x/multisig/handlers_test.go +++ b/x/multisig/handlers_test.go @@ -195,7 +195,7 @@ func TestUpdateContractMsgHandler(t *testing.T) { AdminThreshold: 5, }, signers: []weave.Condition{a}, - err: errors.ErrUnauthorized.Newf("contract=%X", mutableID), + err: errors.ErrUnauthorized.Newf("contract=%X", mutableID), }, { name: "immutable", diff --git a/x/nft/approvals.go b/x/nft/approvals.go index 2d9c4ac9..ec0815aa 100644 --- a/x/nft/approvals.go +++ b/x/nft/approvals.go @@ -69,7 +69,7 @@ func (a ApprovalOptions) EqualsAfterUse(used ApprovalOptions) bool { func (a ApprovalOptions) Validate() error { if a.Count == 0 || a.Count < UnlimitedCount { - return errors.ErrInternalLegacy("Approval count should either be unlimited or above zero") + return errors.ErrInternal.New("Approval count should either be unlimited or above zero") } return nil } @@ -84,11 +84,11 @@ func (m Approvals) Validate(actionMaps ...map[Action]int32) error { } if !isValidAction(action) { - return errors.ErrInternalLegacy(fmt.Sprintf("illegal action: %s", action)) + return errors.ErrInternal.New(fmt.Sprintf("illegal action: %s", action)) } for _, actionMap := range actionMaps { if _, ok := actionMap[action]; ok { - return errors.ErrInternalLegacy(fmt.Sprintf("illegal action: %s", action)) + return errors.ErrInternal.New(fmt.Sprintf("illegal action: %s", action)) } } } diff --git a/x/nft/msg.go b/x/nft/msg.go index d804f5f3..04ceefd6 100644 --- a/x/nft/msg.go +++ b/x/nft/msg.go @@ -28,10 +28,10 @@ func (m AddApprovalMsg) Validate() error { return err } if !isValidAction(m.Action) { - return errors.ErrInternalLegacy("invalid action") + return errors.ErrInternal.New("invalid action") } if !isValidTokenID(m.ID) { - return errors.ErrInternalLegacy("invalid token ID") + return errors.ErrInternal.New("invalid token ID") } return m.Options.Validate() } @@ -41,10 +41,10 @@ func (m RemoveApprovalMsg) Validate() error { return err } if !isValidAction(m.Action) { - return errors.ErrInternalLegacy("invalid action") + return errors.ErrInternal.New("invalid action") } if !isValidTokenID(m.ID) { - return errors.ErrInternalLegacy("invalid token ID") + return errors.ErrInternal.New("invalid token ID") } return nil } From e49f08e67c8ed4e8780b4541c9dcbda5a429e568 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 15:44:15 +0100 Subject: [PATCH 10/22] more common errors refactored --- cmd/bnsd/x/nft/username/handler.go | 6 +++--- errors/common.go | 24 ----------------------- examples/errors/demo.go | 2 +- examples/tutorial/x/blog/handlers.go | 10 +++++----- examples/tutorial/x/blog/handlers_test.go | 4 ++-- x/cash/handler.go | 4 ++-- x/cash/handler_test.go | 2 +- x/currency/handler.go | 2 +- x/escrow/handler.go | 8 ++++---- x/multisig/handlers.go | 4 ++-- x/namecoin/handler.go | 4 ++-- x/namecoin/handler_test.go | 6 +++--- x/nft/base/handler.go | 2 +- x/validators/handler.go | 2 +- x/validators/handler_test.go | 4 ++-- 15 files changed, 30 insertions(+), 54 deletions(-) diff --git a/cmd/bnsd/x/nft/username/handler.go b/cmd/bnsd/x/nft/username/handler.go index 51a0319c..1ad6f7c5 100644 --- a/cmd/bnsd/x/nft/username/handler.go +++ b/cmd/bnsd/x/nft/username/handler.go @@ -74,7 +74,7 @@ func (h IssueHandler) validate(ctx weave.Context, tx weave.Tx) (*IssueTokenMsg, } msg, ok := rmsg.(*IssueTokenMsg) if !ok { - return nil, errors.ErrUnknownTxType(rmsg) + return nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } if err := msg.Validate(); err != nil { return nil, err @@ -139,7 +139,7 @@ func (h *AddChainAddressHandler) validate(ctx weave.Context, tx weave.Tx) (*AddC } msg, ok := rmsg.(*AddChainAddressMsg) if !ok { - return nil, errors.ErrUnknownTxType(rmsg) + return nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } if err := msg.Validate(); err != nil { return nil, err @@ -205,7 +205,7 @@ func (h *RemoveChainAddressHandler) validate(ctx weave.Context, tx weave.Tx) (*R } msg, ok := rmsg.(*RemoveChainAddressMsg) if !ok { - return nil, errors.ErrUnknownTxType(rmsg) + return nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } if err := msg.Validate(); err != nil { return nil, err diff --git a/errors/common.go b/errors/common.go index fa95d360..55b6322d 100644 --- a/errors/common.go +++ b/errors/common.go @@ -75,18 +75,6 @@ func Recover(err *error) { } } -// ErrUnknownTxType creates an error for unexpected transaction objects -func ErrUnknownTxType(tx interface{}) error { - msg := fmt.Sprintf("%T", tx) - return WithLog(msg, errUnknownTxType, CodeUnknownRequest) -} - -// IsUnknownTxTypeErr returns true if an error was created with -// ErrUnknownTxType -func IsUnknownTxTypeErr(err error) bool { - return IsSameError(errUnknownTxType, err) -} - // ErrUnrecognizedAddress may be used for empty addresses, or // badly formatted addresses func ErrUnrecognizedAddress(addr []byte) error { @@ -129,12 +117,6 @@ func ErrTooLarge() error { return WithCode(errTooLarge, CodeTxParseError) } -// IsTooLargeErr returns true iff an error was created -// with ErrTooLarge -func IsTooLargeErr(err error) bool { - return IsSameError(errTooLarge, err) -} - // IsUnauthorizedErr is generic helper for any unauthorized errors, // also specific sub-types func IsUnauthorizedErr(err error) bool { @@ -146,12 +128,6 @@ func ErrMissingSignature() error { return WithCode(errMissingSignature, CodeUnauthorized) } -// IsMissingSignatureErr returns true iff an error was created -// with ErrMissingSignature -func IsMissingSignatureErr(err error) bool { - return IsSameError(errMissingSignature, err) -} - // ErrInvalidSignature is when the signature doesn't match // (bad key, bad nonce, bad chainID) func ErrInvalidSignature() error { diff --git a/examples/errors/demo.go b/examples/errors/demo.go index 0a49746c..8ecaca87 100644 --- a/examples/errors/demo.go +++ b/examples/errors/demo.go @@ -24,7 +24,7 @@ type foo struct { } func fullError() error { - return errors.ErrUnknownTxType(&foo{7}) + return errors.WithType(errors.ErrInvalidMsg, &foo{7}) } func panicError() (err error) { diff --git a/examples/tutorial/x/blog/handlers.go b/examples/tutorial/x/blog/handlers.go index 9f3e4328..9f6cdc90 100644 --- a/examples/tutorial/x/blog/handlers.go +++ b/examples/tutorial/x/blog/handlers.go @@ -100,7 +100,7 @@ func (h CreateBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w createBlogMsg, ok := msg.(*CreateBlogMsg) if !ok { - return nil, errors.ErrUnknownTxType(msg) + return nil, errors.WithType(errors.ErrInvalidMsg, msg) } err = createBlogMsg.Validate() @@ -179,7 +179,7 @@ func (h CreatePostMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w createPostMsg, ok := msg.(*CreatePostMsg) if !ok { - return nil, nil, errors.ErrUnknownTxType(msg) + return nil, nil, errors.WithType(errors.ErrInvalidMsg, msg) } // Check the author is one of the Tx signer @@ -262,7 +262,7 @@ func (h RenameBlogMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w renameBlogMsg, ok := msg.(*RenameBlogMsg) if !ok { - return nil, nil, errors.ErrUnknownTxType(msg) + return nil, nil, errors.WithType(errors.ErrInvalidMsg, msg) } err = renameBlogMsg.Validate() @@ -355,7 +355,7 @@ func (h ChangeBlogAuthorsMsgHandler) validate(ctx weave.Context, db weave.KVStor changeBlogAuthorsMsg, ok := msg.(*ChangeBlogAuthorsMsg) if !ok { - return nil, nil, errors.ErrUnknownTxType(msg) + return nil, nil, errors.WithType(errors.ErrInvalidMsg, msg) } err = changeBlogAuthorsMsg.Validate() @@ -462,7 +462,7 @@ func (h SetProfileMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx w setProfileMsg, ok := msg.(*SetProfileMsg) if !ok { - return nil, nil, errors.ErrUnknownTxType(msg) + return nil, nil, errors.WithType(errors.ErrInvalidMsg, msg) } // if author is here we use it for authentication diff --git a/examples/tutorial/x/blog/handlers_test.go b/examples/tutorial/x/blog/handlers_test.go index 22c388ae..580e56a8 100644 --- a/examples/tutorial/x/blog/handlers_test.go +++ b/examples/tutorial/x/blog/handlers_test.go @@ -289,7 +289,7 @@ func TestCreateBlogMsgHandlerCheck(t *testing.T) { }, { Name: "wrong msg type", - Err: errors.ErrUnknownTxType(&CreatePostMsg{ + Err: errors.WithType(errors.ErrInvalidMsg, &CreatePostMsg{ Blog: "this_is_a_blog", Title: "this is a post title", Text: longText, @@ -458,7 +458,7 @@ func TestCreatePostMsgHandlerCheck(t *testing.T) { }, { Name: "wrong msg type", - Err: errors.ErrUnknownTxType(&CreateBlogMsg{ + Err: errors.WithType(errors.ErrInvalidMsg, &CreateBlogMsg{ Slug: "this_is_a_blog", Title: "this is a blog title", Authors: [][]byte{signer.Address()}, diff --git a/x/cash/handler.go b/x/cash/handler.go index 5506a04d..f924e817 100644 --- a/x/cash/handler.go +++ b/x/cash/handler.go @@ -48,7 +48,7 @@ func (h SendHandler) Check(ctx weave.Context, store weave.KVStore, } msg, ok := rmsg.(*SendMsg) if !ok { - return res, errors.ErrUnknownTxType(rmsg) + return res, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() @@ -79,7 +79,7 @@ func (h SendHandler) Deliver(ctx weave.Context, store weave.KVStore, } msg, ok := rmsg.(*SendMsg) if !ok { - return res, errors.ErrUnknownTxType(rmsg) + return res, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() diff --git a/x/cash/handler_test.go b/x/cash/handler_test.go index cfea1922..8b31cb25 100644 --- a/x/cash/handler_test.go +++ b/x/cash/handler_test.go @@ -33,7 +33,7 @@ func TestSend(t *testing.T) { expectCheck checkErr expectDeliver checkErr }{ - 0: {nil, nil, nil, errors.IsUnknownTxTypeErr, errors.IsUnknownTxTypeErr}, + 0: {nil, nil, nil, errors.ErrInvalidMsg.Is, errors.ErrInvalidMsg.Is}, 1: {nil, nil, new(SendMsg), errors.ErrInvalidAmount.Is, errors.ErrInvalidAmount.Is}, 2: {nil, nil, &SendMsg{Amount: &foo}, errors.IsUnrecognizedAddressErr, errors.IsUnrecognizedAddressErr}, 3: { diff --git a/x/currency/handler.go b/x/currency/handler.go index 19225c3b..0499e8b4 100644 --- a/x/currency/handler.go +++ b/x/currency/handler.go @@ -56,7 +56,7 @@ func (h *TokenInfoHandler) validate(ctx weave.Context, db weave.KVStore, tx weav } msg, ok := rmsg.(*NewTokenInfoMsg) if !ok { - return nil, errors.ErrUnknownTxType(rmsg) + return nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } if err := msg.Validate(); err != nil { diff --git a/x/escrow/handler.go b/x/escrow/handler.go index e86c5229..32efe454 100644 --- a/x/escrow/handler.go +++ b/x/escrow/handler.go @@ -107,7 +107,7 @@ func (h CreateEscrowHandler) validate(ctx weave.Context, db weave.KVStore, } msg, ok := rmsg.(*CreateEscrowMsg) if !ok { - return nil, errors.ErrUnknownTxType(rmsg) + return nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() @@ -205,7 +205,7 @@ func (h ReleaseEscrowHandler) validate(ctx weave.Context, db weave.KVStore, } msg, ok := rmsg.(*ReleaseEscrowMsg) if !ok { - return nil, nil, errors.ErrUnknownTxType(rmsg) + return nil, nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() @@ -289,7 +289,7 @@ func (h ReturnEscrowHandler) validate(ctx weave.Context, db weave.KVStore, } msg, ok := rmsg.(*ReturnEscrowMsg) if !ok { - return nil, nil, errors.ErrUnknownTxType(rmsg) + return nil, nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() @@ -377,7 +377,7 @@ func (h UpdateEscrowHandler) validate(ctx weave.Context, db weave.KVStore, } msg, ok := rmsg.(*UpdateEscrowPartiesMsg) if !ok { - return nil, nil, errors.ErrUnknownTxType(rmsg) + return nil, nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() diff --git a/x/multisig/handlers.go b/x/multisig/handlers.go index 4f933952..fa74699b 100644 --- a/x/multisig/handlers.go +++ b/x/multisig/handlers.go @@ -74,7 +74,7 @@ func (h CreateContractMsgHandler) validate(ctx weave.Context, db weave.KVStore, createContractMsg, ok := msg.(*CreateContractMsg) if !ok { - return nil, errors.ErrUnknownTxType(msg) + return nil, errors.WithType(errors.ErrInvalidMsg, msg) } err = createContractMsg.Validate() @@ -134,7 +134,7 @@ func (h UpdateContractMsgHandler) validate(ctx weave.Context, db weave.KVStore, updateContractMsg, ok := msg.(*UpdateContractMsg) if !ok { - return nil, errors.ErrUnknownTxType(msg) + return nil, errors.WithType(errors.ErrInvalidMsg, msg) } err = updateContractMsg.Validate() diff --git a/x/namecoin/handler.go b/x/namecoin/handler.go index e3ef2307..ca3da507 100644 --- a/x/namecoin/handler.go +++ b/x/namecoin/handler.go @@ -106,7 +106,7 @@ func (h TokenHandler) validate(ctx weave.Context, db weave.KVStore, } msg, ok := rmsg.(*NewTokenMsg) if !ok { - return nil, errors.ErrUnknownTxType(rmsg) + return nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() @@ -192,7 +192,7 @@ func (h SetNameHandler) validate(ctx weave.Context, db weave.KVStore, } msg, ok := rmsg.(*SetWalletNameMsg) if !ok { - return nil, errors.ErrUnknownTxType(rmsg) + return nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() diff --git a/x/namecoin/handler_test.go b/x/namecoin/handler_test.go index c53f7873..53079182 100644 --- a/x/namecoin/handler_test.go +++ b/x/namecoin/handler_test.go @@ -46,7 +46,7 @@ func TestSendHandler(t *testing.T) { expectCheck checkErr expectDeliver checkErr }{ - 0: {nil, nil, nil, errors.IsUnknownTxTypeErr, errors.IsUnknownTxTypeErr}, + 0: {nil, nil, nil, errors.ErrInvalidMsg.Is, errors.ErrInvalidMsg.Is}, 1: {nil, nil, new(cash.SendMsg), errors.ErrInvalidAmount.Is, errors.ErrInvalidAmount.Is}, 2: {nil, nil, &cash.SendMsg{Amount: &foo}, errors.IsUnrecognizedAddressErr, errors.IsUnrecognizedAddressErr}, 3: { @@ -132,7 +132,7 @@ func TestNewTokenHandler(t *testing.T) { }{ // wrong message type 0: {nil, nil, nil, new(cash.SendMsg), - errors.IsUnknownTxTypeErr, errors.IsUnknownTxTypeErr, "", nil}, + errors.ErrInvalidMsg.Is, errors.ErrInvalidMsg.Is, "", nil}, // wrong currency values 1: {nil, nil, nil, BuildTokenMsg("YO", "digga", 7), x.ErrInvalidCurrency.Is, x.ErrInvalidCurrency.Is, "", nil}, @@ -218,7 +218,7 @@ func TestSetNameHandler(t *testing.T) { }{ // wrong message type 0: {nil, nil, new(cash.SendMsg), - errors.IsUnknownTxTypeErr, errors.IsUnknownTxTypeErr, nil, nil}, + errors.ErrInvalidMsg.Is, errors.ErrInvalidMsg.Is, nil, nil}, // invalid message 1: {nil, nil, BuildSetNameMsg([]byte{1, 2}, "johnny"), errors.IsUnrecognizedAddressErr, errors.IsUnrecognizedAddressErr, nil, nil}, diff --git a/x/nft/base/handler.go b/x/nft/base/handler.go index b15d9b35..e4a69a7f 100644 --- a/x/nft/base/handler.go +++ b/x/nft/base/handler.go @@ -126,6 +126,6 @@ func (h *ApprovalOpsHandler) validate(ctx weave.Context, tx weave.Tx) (nft.Appro } return v.(nft.ApprovalMsg), nil default: - return nil, errors.ErrUnknownTxType(msg) + return nil, errors.WithType(errors.ErrInvalidMsg, msg) } } diff --git a/x/validators/handler.go b/x/validators/handler.go index 0b758a9a..e1b8832c 100644 --- a/x/validators/handler.go +++ b/x/validators/handler.go @@ -75,7 +75,7 @@ func (h UpdateHandler) validate(ctx weave.Context, store weave.KVStore, tx weave } msg, ok := rmsg.(*SetValidatorsMsg) if !ok { - return nil, errors.ErrUnknownTxType(rmsg) + return nil, errors.WithType(errors.ErrInvalidMsg, rmsg) } err = msg.Validate() if err != nil { diff --git a/x/validators/handler_test.go b/x/validators/handler_test.go index 83077faa..256f33a5 100644 --- a/x/validators/handler_test.go +++ b/x/validators/handler_test.go @@ -70,10 +70,10 @@ func TestHandler(t *testing.T) { handler := NewUpdateHandler(auth2, ctrl, authCheckAddress) _, err := handler.Deliver(nil, kv, tx) - So(err.Error(), ShouldResemble, errors.ErrUnknownTxType(msg).Error()) + So(err.Error(), ShouldResemble, errors.WithType(errors.ErrInvalidMsg, msg).Error()) _, err = handler.Check(nil, kv, tx) - So(err.Error(), ShouldResemble, errors.ErrUnknownTxType(msg).Error()) + So(err.Error(), ShouldResemble, errors.WithType(errors.ErrInvalidMsg, msg).Error()) }) }) }) From 90748be834e888cf32aa25184425f2c130cf82f3 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 16:28:44 +0100 Subject: [PATCH 11/22] more refactoring --- abci_test.go | 14 ++- app/commit.go | 4 +- app/router.go | 4 +- app/store.go | 6 +- conditions.go | 7 +- errors/common.go | 106 +--------------------- errors/errors_test.go | 8 +- errors/main.go | 15 --- examples/errors/demo.go | 2 +- examples/mycoind/app/tx.go | 2 +- examples/tutorial/x/blog/handlers_test.go | 4 +- x/batch/msg.go | 2 +- x/cash/decorator_test.go | 2 +- x/cash/handler_test.go | 2 +- x/cash/msg.go | 2 +- x/cash/msg_test.go | 6 +- x/distribution/model_test.go | 4 +- x/escrow/errors.go | 4 +- x/namecoin/handler_test.go | 4 +- x/namecoin/init.go | 2 +- x/namecoin/msg.go | 2 +- x/sigs/controller.go | 4 +- x/sigs/decorator.go | 4 +- x/sigs/errors.go | 9 +- x/sigs/tx.go | 2 +- x/utils/recover_test.go | 4 +- 26 files changed, 51 insertions(+), 174 deletions(-) diff --git a/abci_test.go b/abci_test.go index 31249e49..98ac8486 100644 --- a/abci_test.go +++ b/abci_test.go @@ -16,14 +16,12 @@ func TestCreateErrorResult(t *testing.T) { msg string code uint32 }{ - {errors.NormalizePanic("stdlib"), "internal", errors.CodeInternalErr}, - {fmt.Errorf("base"), "base", errors.CodeInternalErr}, - {pkerr.New("dave"), "dave", errors.CodeInternalErr}, - {errors.Wrap(fmt.Errorf("demo"), "wrapped"), "wrapped: demo", errors.CodeInternalErr}, - {errors.New(fmt.Errorf("stdlib").Error(), 5), "stdlib", 5}, - {errors.New("nonce", errors.CodeUnauthorized), "nonce", errors.CodeUnauthorized}, - {errors.WithCode(fmt.Errorf("no sender"), errors.CodeUnrecognizedAddress), "no sender", errors.CodeUnrecognizedAddress}, - {errors.ErrDecoding(), errors.ErrDecoding().Error(), errors.CodeTxParseError}, + {errors.NormalizePanic("stdlib"), "internal", errors.ErrInternal.ABCICode()}, + {fmt.Errorf("base"), "base", errors.ErrInternal.ABCICode()}, + {pkerr.New("dave"), "dave", errors.ErrInternal.ABCICode()}, + {errors.Wrap(fmt.Errorf("demo"), "wrapped"), "wrapped: demo", errors.ErrInternal.ABCICode()}, + {errors.WithCode(fmt.Errorf("no sender"), 11), "no sender", 11}, + {errors.ErrInvalidInput.New("unable to decode"), errors.ErrInvalidInput.New("unable to decode").Error(), errors.ErrInvalidInput.ABCICode()}, } for i, tc := range cases { diff --git a/app/commit.go b/app/commit.go index fce7cdb8..28fa31a1 100644 --- a/app/commit.go +++ b/app/commit.go @@ -79,11 +79,11 @@ func loadChainID(kv weave.KVStore) string { // Returns error if already set, or invalid name func saveChainID(kv weave.KVStore, chainID string) error { if !weave.IsValidChainID(chainID) { - return errors.ErrInvalidChainID(chainID) + return errors.ErrInvalidInput.Newf("chain id: %v", chainID) } k := []byte(chainIDKey) if kv.Has(k) { - return errors.ErrModifyChainID() + return errors.ErrUnauthorized.New("can't modify chain id after genesis init") } kv.Set(k, []byte(chainID)) return nil diff --git a/app/router.go b/app/router.go index be9bbd7f..9f5e81e2 100644 --- a/app/router.go +++ b/app/router.go @@ -82,7 +82,7 @@ func (r Router) Check(ctx weave.Context, store weave.KVStore, msg, _ := tx.GetMsg() if msg == nil { - return weave.CheckResult{}, errors.ErrDecoding() + return weave.CheckResult{}, errors.ErrInvalidInput.New("unable to decode") } path := msg.Path() h := r.Handler(path) @@ -95,7 +95,7 @@ func (r Router) Deliver(ctx weave.Context, store weave.KVStore, msg, _ := tx.GetMsg() if msg == nil { - return weave.DeliverResult{}, errors.ErrDecoding() + return weave.DeliverResult{}, errors.ErrInvalidInput.New("unable to decode") } path := msg.Path() h := r.Handler(path) diff --git a/app/store.go b/app/store.go index 98db1d76..eaa5e154 100644 --- a/app/store.go +++ b/app/store.go @@ -111,7 +111,7 @@ func (s *StoreApp) parseAppState(data []byte, chainID string, init weave.Initial var appState weave.Options err := json.Unmarshal(data, &appState) if err != nil { - return errors.WithCode(err, errors.CodeTxParseError) + return errors.ErrInternal.New(err.Error()) } err = s.storeChainID(chainID) @@ -215,7 +215,7 @@ func (s *StoreApp) Query(reqQuery abci.RequestQuery) (resQuery abci.ResponseQuer path, mod := splitPath(reqQuery.Path) qh := s.queryRouter.Handler(path) if qh == nil { - resQuery.Code = errors.CodeUnknownRequest + resQuery.Code = errors.ErrNotFound.ABCICode() resQuery.Log = fmt.Sprintf("Unexpected Query path: %v", reqQuery.Path) return } @@ -279,7 +279,7 @@ func splitPath(path string) (string, string) { func queryError(err error) abci.ResponseQuery { return abci.ResponseQuery{ Log: err.Error(), - Code: errors.CodeInternalErr, + Code: errors.ErrInternal.ABCICode(), } } diff --git a/conditions.go b/conditions.go index 530872af..238b9d08 100644 --- a/conditions.go +++ b/conditions.go @@ -38,7 +38,8 @@ func NewCondition(ext, typ string, data []byte) Condition { func (c Condition) Parse() (string, string, []byte, error) { chunks := perm.FindSubmatch(c) if len(chunks) == 0 { - return "", "", nil, errors.ErrUnrecognizedCondition(c) + return "", "", nil, errors.ErrInvalidInput.Newf("condition: %X", []byte(c)) + } // returns [all, match1, match2, match3] return string(chunks[1]), string(chunks[2]), chunks[3], nil @@ -68,7 +69,7 @@ func (c Condition) String() string { // Validate returns an error if the Condition is not the proper format func (c Condition) Validate() error { if !perm.Match(c) { - return errors.ErrUnrecognizedCondition(c) + return errors.ErrInvalidInput.Newf("condition: %X", []byte(c)) } return nil } @@ -109,7 +110,7 @@ func (a Address) String() string { // Validate returns an error if the address is not the valid size func (a Address) Validate() error { if len(a) != AddressLength { - return errors.ErrUnrecognizedAddress(a) + return errors.ErrInvalidInput.Newf("address: %v", a) } return nil } diff --git a/errors/common.go b/errors/common.go index 55b6322d..38ed182b 100644 --- a/errors/common.go +++ b/errors/common.go @@ -1,36 +1,11 @@ package errors import ( - stderrors "errors" "fmt" "github.com/pkg/errors" ) -// ABCI Response Codes -// Base SDK reserves 0 ~ 99. -const ( - CodeInternalErr uint32 = 1 - CodeTxParseError = 2 - CodeUnauthorized = 3 - CodeUnknownRequest = 4 - CodeUnrecognizedAddress = 5 - CodeInvalidChainID = 6 - CodePanic = 111222 // TODO: use maxint or such? -) - -var ( - errDecoding = stderrors.New("Error decoding input") - errTooLarge = stderrors.New("Input size too large") - errUnknownTxType = stderrors.New("Tx type unknown") - errMissingSignature = stderrors.New("Signature missing") - errInvalidSignature = stderrors.New("Signature invalid") - errUnrecognizedAddress = stderrors.New("Unrecognized Address") - errUnrecognizedCondition = stderrors.New("Unrecognized Condition") - errInvalidChainID = stderrors.New("Invalid ChainID") - errModifyChainID = stderrors.New("Cannot modify ChainID") -) - // IsSameError returns true if these errors have the same root cause. // pattern is the expected error type and should always be non-nil // err may be anything and returns true if it is a wrapped version of pattern @@ -43,7 +18,7 @@ func HasErrorCode(err error, code uint32) bool { if tm, ok := err.(coder); ok { return tm.ABCICode() == code } - return code == CodeInternalErr + return code == ErrInternal.ABCICode() } // NormalizePanic converts a panic into a redacted error @@ -55,8 +30,7 @@ func NormalizePanic(p interface{}) error { // if err, isErr := p.(error); isErr { // return Wrap(err, "normalized panic") // } - msg := fmt.Sprintf("panic: %v", p) - return Error{code: CodePanic, desc: msg} + return ErrPanic.Newf("%v", p) } // Redact will replace all panic errors with a generic message @@ -75,82 +49,6 @@ func Recover(err *error) { } } -// ErrUnrecognizedAddress may be used for empty addresses, or -// badly formatted addresses -func ErrUnrecognizedAddress(addr []byte) error { - msg := "(nil)" - if len(addr) > 0 { - msg = fmt.Sprintf("%X", addr) - } - return WithLog(msg, errUnrecognizedAddress, CodeUnrecognizedAddress) -} - -// IsUnrecognizedAddressErr returns true iff an error was created -// with ErrUnrecognizedAddress -func IsUnrecognizedAddressErr(err error) bool { - return IsSameError(errUnrecognizedAddress, err) -} - -// ErrUnrecognizedCondition is used for anything that is not -// the proper format -func ErrUnrecognizedCondition(cond []byte) error { - msg := "(nil)" - if len(cond) > 0 { - msg = fmt.Sprintf("%X", cond) - } - return WithLog(msg, errUnrecognizedCondition, CodeUnrecognizedAddress) -} - -// IsUnrecognizedConditionErr returns true iff an error was created -// with ErrUnrecognizedCondition -func IsUnrecognizedConditionErr(err error) bool { - return IsSameError(errUnrecognizedCondition, err) -} - -// ErrDecoding is generic when we cannot parse the transaction input -func ErrDecoding() error { - return WithCode(errDecoding, CodeTxParseError) -} - -// ErrTooLarge is a specific decode error when we pass the max tx size -func ErrTooLarge() error { - return WithCode(errTooLarge, CodeTxParseError) -} - -// IsUnauthorizedErr is generic helper for any unauthorized errors, -// also specific sub-types -func IsUnauthorizedErr(err error) bool { - return HasErrorCode(err, CodeUnauthorized) -} - -// ErrMissingSignature is returned when no signature is present -func ErrMissingSignature() error { - return WithCode(errMissingSignature, CodeUnauthorized) -} - -// ErrInvalidSignature is when the signature doesn't match -// (bad key, bad nonce, bad chainID) -func ErrInvalidSignature() error { - return WithCode(errInvalidSignature, CodeUnauthorized) -} - -// IsInvalidSignatureErr returns true iff an error was created -// with ErrInvalidSignature -func IsInvalidSignatureErr(err error) bool { - return IsSameError(errInvalidSignature, err) -} - -// ErrInvalidChainID is when the chainID is the wrong format -func ErrInvalidChainID(chainID string) error { - return WithLog(chainID, errInvalidChainID, CodeInvalidChainID) -} - -// ErrModifyChainID is when someone tries to change the chainID -// after genesis -func ErrModifyChainID() error { - return WithCode(errModifyChainID, CodeInvalidChainID) -} - func WithType(err error, obj interface{}) error { return Wrap(err, fmt.Sprintf("%T", obj)) } diff --git a/errors/errors_test.go b/errors/errors_test.go index a0af6817..91a0ff7d 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -47,14 +47,14 @@ func TestErrors(t *testing.T) { "normalize panic handles strings": { err: NormalizePanic("foo"), wantRoot: ErrPanic, - wantMsg: "panic: foo", - wantLog: "panic: foo", + wantMsg: "foo: panic", + wantLog: "foo: panic", }, "normalize panic handles errors": { err: NormalizePanic(fmt.Errorf("message")), wantRoot: ErrPanic, - wantMsg: "panic: message", - wantLog: "panic: message", + wantMsg: "message: panic", + wantLog: "message: panic", }, } diff --git a/errors/main.go b/errors/main.go index 8750b455..015ad4d3 100644 --- a/errors/main.go +++ b/errors/main.go @@ -16,21 +16,6 @@ type TMError interface { ABCILog() string } -// This function is deprecated. Error codes are no longer part of an error API. -// -// New creates an error with the given message and a stacktrace, -// and sets the code and log, -// overriding the state if err was already TMError -func New(log string, code uint32) error { - // create a new error with stack trace and attach a code - st := errors.New(log).(stackTracer) - return tmerror{ - stackTracer: st, - code: code, - log: log, - } -} - // WithCode adds a stacktrace if necessary and sets the code and msg, // overriding the code if err was already TMError func WithCode(err error, code uint32) TMError { diff --git a/examples/errors/demo.go b/examples/errors/demo.go index 8ecaca87..6e9f2ee4 100644 --- a/examples/errors/demo.go +++ b/examples/errors/demo.go @@ -16,7 +16,7 @@ func makeError() error { } func otherError() error { - return errors.ErrDecoding() + return errors.ErrInvalidInput.New("unable to decode") } type foo struct { diff --git a/examples/mycoind/app/tx.go b/examples/mycoind/app/tx.go index 32f4a15e..33294936 100644 --- a/examples/mycoind/app/tx.go +++ b/examples/mycoind/app/tx.go @@ -26,7 +26,7 @@ var _ sigs.SignedTx = (*Tx)(nil) func (tx *Tx) GetMsg() (weave.Msg, error) { sum := tx.GetSum() if sum == nil { - return nil, errors.ErrDecoding() + return nil, errors.ErrInvalidInput.New("unable to decode") } // make sure to cover all messages defined in protobuf diff --git a/examples/tutorial/x/blog/handlers_test.go b/examples/tutorial/x/blog/handlers_test.go index 580e56a8..93a1ee32 100644 --- a/examples/tutorial/x/blog/handlers_test.go +++ b/examples/tutorial/x/blog/handlers_test.go @@ -746,7 +746,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "adding with no author", - Err: errors.ErrUnrecognizedAddress(nil), + Err: errors.ErrInvalidInput.Newf("address: %v", nil), Handler: changeBlogAuthorsMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &ChangeBlogAuthorsMsg{ @@ -756,7 +756,7 @@ func TestChangeBlogAuthorsMsgHandlerCheck(t *testing.T) { }, { Name: "removing with no author", - Err: errors.ErrUnrecognizedAddress(nil), + Err: errors.ErrInvalidInput.Newf("address: %v", nil), Handler: changeBlogAuthorsMsgHandlerFn, Perms: []weave.Condition{signer}, Msg: &ChangeBlogAuthorsMsg{ diff --git a/x/batch/msg.go b/x/batch/msg.go index e992545c..c052f876 100644 --- a/x/batch/msg.go +++ b/x/batch/msg.go @@ -23,7 +23,7 @@ func Validate(msg Msg) error { } if len(l) > MaxBatchMessages { - return errors.ErrTooLarge() + return errors.ErrInvalidInput.New("transaction is too large") } return nil } diff --git a/x/cash/decorator_test.go b/x/cash/decorator_test.go index d41c03a3..fe927677 100644 --- a/x/cash/decorator_test.go +++ b/x/cash/decorator_test.go @@ -79,7 +79,7 @@ func TestFees(t *testing.T) { // no fee given, something expected 1: {nil, nil, nil, min, errors.ErrInsufficientAmount.Is}, // no signer given - 2: {nil, nil, &FeeInfo{Fees: &min}, min, errors.IsUnrecognizedAddressErr}, + 2: {nil, nil, &FeeInfo{Fees: &min}, min, errors.ErrInvalidInput.Is}, // use default signer, but not enough money 3: { []weave.Condition{perm}, diff --git a/x/cash/handler_test.go b/x/cash/handler_test.go index 8b31cb25..0c39a6bd 100644 --- a/x/cash/handler_test.go +++ b/x/cash/handler_test.go @@ -35,7 +35,7 @@ func TestSend(t *testing.T) { }{ 0: {nil, nil, nil, errors.ErrInvalidMsg.Is, errors.ErrInvalidMsg.Is}, 1: {nil, nil, new(SendMsg), errors.ErrInvalidAmount.Is, errors.ErrInvalidAmount.Is}, - 2: {nil, nil, &SendMsg{Amount: &foo}, errors.IsUnrecognizedAddressErr, errors.IsUnrecognizedAddressErr}, + 2: {nil, nil, &SendMsg{Amount: &foo}, errors.ErrInvalidInput.Is, errors.ErrInvalidInput.Is}, 3: { nil, nil, diff --git a/x/cash/msg.go b/x/cash/msg.go index 5a6e5d30..4b31ef39 100644 --- a/x/cash/msg.go +++ b/x/cash/msg.go @@ -87,7 +87,7 @@ func (f *FeeInfo) DefaultPayer(addr []byte) *FeeInfo { // Note that fee must be present, even if 0 func (f *FeeInfo) Validate() error { if f == nil { - return errors.ErrUnrecognizedAddress(nil) + return errors.ErrInvalidInput.Newf("address: %v", nil) } fee := f.GetFees() if fee == nil { diff --git a/x/cash/msg_test.go b/x/cash/msg_test.go index 31229a36..e46d5e70 100644 --- a/x/cash/msg_test.go +++ b/x/cash/msg_test.go @@ -26,7 +26,7 @@ func TestValidateSendMsg(t *testing.T) { } err := noSrc.Validate() assert.Error(t, err) - assert.True(t, errors.IsUnrecognizedAddressErr(err)) + assert.True(t, errors.ErrInvalidInput.Is(err)) // add a default source, so it validates good := noSrc.DefaultSource(addr2) @@ -87,7 +87,7 @@ func TestValidateFeeTx(t *testing.T) { var empty *FeeInfo err := empty.Validate() assert.Error(t, err) - assert.True(t, errors.IsUnrecognizedAddressErr(err)) + assert.True(t, errors.ErrInvalidInput.Is(err)) addr := weave.NewAddress([]byte{8, 8}) addr2 := weave.NewAddress([]byte{7, 7}) @@ -101,7 +101,7 @@ func TestValidateFeeTx(t *testing.T) { plus := &FeeInfo{Fees: &pos} err = plus.Validate() assert.Error(t, err) - assert.True(t, errors.IsUnrecognizedAddressErr(err)) + assert.True(t, errors.ErrInvalidInput.Is(err)) full := plus.DefaultPayer(addr) assert.NoError(t, full.Validate()) diff --git a/x/distribution/model_test.go b/x/distribution/model_test.go index e940ce46..908e0fcb 100644 --- a/x/distribution/model_test.go +++ b/x/distribution/model_test.go @@ -32,7 +32,7 @@ func TestRevenueValidate(t *testing.T) { {Weight: 1, Address: addr}, }, }, - wantErr: errors.ErrInvalidModel, + wantErr: errors.ErrInvalidInput, }, "at least one recipient must be given": { model: Revenue{ @@ -57,7 +57,7 @@ func TestRevenueValidate(t *testing.T) { {Weight: 2, Address: []byte("zzz")}, }, }, - wantErr: errors.ErrInvalidModel, + wantErr: errors.ErrInvalidInput, }, } diff --git a/x/escrow/errors.go b/x/escrow/errors.go index 6a1c120e..29aa6f18 100644 --- a/x/escrow/errors.go +++ b/x/escrow/errors.go @@ -34,10 +34,10 @@ var ( ) func ErrInvalidCondition(perm []byte) error { - return errors.ErrUnrecognizedCondition(perm) + return errors.ErrInvalidInput.Newf("condition: %v", perm) } func IsInvalidConditionErr(err error) bool { - return errors.IsUnrecognizedConditionErr(err) + return errors.ErrInvalidInput.Is(err) } func ErrInvalidMemo(memo string) error { diff --git a/x/namecoin/handler_test.go b/x/namecoin/handler_test.go index 53079182..5dbe86eb 100644 --- a/x/namecoin/handler_test.go +++ b/x/namecoin/handler_test.go @@ -48,7 +48,7 @@ func TestSendHandler(t *testing.T) { }{ 0: {nil, nil, nil, errors.ErrInvalidMsg.Is, errors.ErrInvalidMsg.Is}, 1: {nil, nil, new(cash.SendMsg), errors.ErrInvalidAmount.Is, errors.ErrInvalidAmount.Is}, - 2: {nil, nil, &cash.SendMsg{Amount: &foo}, errors.IsUnrecognizedAddressErr, errors.IsUnrecognizedAddressErr}, + 2: {nil, nil, &cash.SendMsg{Amount: &foo}, errors.ErrInvalidInput.Is, errors.ErrInvalidInput.Is}, 3: { nil, nil, @@ -221,7 +221,7 @@ func TestSetNameHandler(t *testing.T) { errors.ErrInvalidMsg.Is, errors.ErrInvalidMsg.Is, nil, nil}, // invalid message 1: {nil, nil, BuildSetNameMsg([]byte{1, 2}, "johnny"), - errors.IsUnrecognizedAddressErr, errors.IsUnrecognizedAddressErr, nil, nil}, + errors.ErrInvalidInput.Is, errors.ErrInvalidInput.Is, nil, nil}, 2: {nil, nil, BuildSetNameMsg(addr, "sh"), IsInvalidWallet, IsInvalidWallet, nil, nil}, // no permission to change account diff --git a/x/namecoin/init.go b/x/namecoin/init.go index 75160d94..40da335c 100644 --- a/x/namecoin/init.go +++ b/x/namecoin/init.go @@ -70,7 +70,7 @@ func setWallets(db weave.KVStore, gens []GenesisAccount) error { bucket := NewWalletBucket() for _, gen := range gens { if len(gen.Address) != weave.AddressLength { - return errors.ErrUnrecognizedAddress(gen.Address) + return errors.ErrInvalidInput.Newf("address: %v", gen.Address) } wallet, err := WalletWith(gen.Address, gen.Name, gen.Wallet.Coins...) if err != nil { diff --git a/x/namecoin/msg.go b/x/namecoin/msg.go index e9eb8a1d..90065e59 100644 --- a/x/namecoin/msg.go +++ b/x/namecoin/msg.go @@ -65,7 +65,7 @@ func (SetWalletNameMsg) Path() string { // Validate makes sure that this is sensible func (s *SetWalletNameMsg) Validate() error { if len(s.Address) != weave.AddressLength { - return errors.ErrUnrecognizedAddress(s.Address) + return errors.ErrInvalidInput.Newf("address: %v", s.Address) } if !IsWalletName(s.Name) { return ErrInvalidWalletName(s.Name) diff --git a/x/sigs/controller.go b/x/sigs/controller.go index f6f80db4..9da9de0d 100644 --- a/x/sigs/controller.go +++ b/x/sigs/controller.go @@ -77,7 +77,7 @@ func VerifySignature(db weave.KVStore, sig *StdSignature, user := AsUser(obj) if !user.Pubkey.Verify(toSign, sig.Signature) { - return nil, errors.ErrInvalidSignature() + return nil, errors.ErrUnauthorized.New("invalid signature") } err = user.CheckAndIncrementSequence(sig.Sequence) @@ -108,7 +108,7 @@ func BuildSignBytes(signBytes []byte, chainID string, seq int64) ([]byte, error) return nil, ErrInvalidSequence("negative") } if !weave.IsValidChainID(chainID) { - return nil, errors.ErrInvalidChainID(chainID) + return nil, errors.ErrInvalidInput.Newf("chain id: %v", chainID) } // encode nonce as 8 byte, big-endian diff --git a/x/sigs/decorator.go b/x/sigs/decorator.go index 5e82155b..b99a0c52 100644 --- a/x/sigs/decorator.go +++ b/x/sigs/decorator.go @@ -58,7 +58,7 @@ func (d Decorator) Check(ctx weave.Context, store weave.KVStore, tx weave.Tx, } } if len(signers) == 0 && !d.allowMissingSigs { - return res, errors.ErrMissingSignature() + return res, errors.ErrUnauthorized.New("missing signature") } ctx = withSigners(ctx, signers) @@ -80,7 +80,7 @@ func (d Decorator) Deliver(ctx weave.Context, store weave.KVStore, tx weave.Tx, } } if len(signers) == 0 && !d.allowMissingSigs { - return res, errors.ErrMissingSignature() + return res, errors.ErrUnauthorized.New("missing signature") } ctx = withSigners(ctx, signers) diff --git a/x/sigs/errors.go b/x/sigs/errors.go index f59fc34c..9ed3a429 100644 --- a/x/sigs/errors.go +++ b/x/sigs/errors.go @@ -30,12 +30,7 @@ func IsInvalidSequenceErr(err error) bool { // all will match IsInvalidSignatureError func ErrMissingPubkey() error { - invalidSig := errors.ErrInvalidSignature() - return errors.WithLog("Missing public key", invalidSig, errors.CodeUnauthorized) -} -func ErrPubkeyAddressMismatch() error { - invalidSig := errors.ErrInvalidSignature() - return errors.WithLog("Pubkey and Address don't match", invalidSig, errors.CodeUnauthorized) + return errors.ErrUnauthorized.New("missing public key") } -var IsInvalidSignatureErr = errors.IsInvalidSignatureErr +var IsInvalidSignatureErr = errors.ErrUnauthorized.Is diff --git a/x/sigs/tx.go b/x/sigs/tx.go index a63371e4..e94d903d 100644 --- a/x/sigs/tx.go +++ b/x/sigs/tx.go @@ -28,7 +28,7 @@ func (s *StdSignature) Validate() error { return ErrMissingPubkey() } if s.Signature == nil { - return errors.ErrMissingSignature() + return errors.ErrUnauthorized.New("missing signature") } return nil diff --git a/x/utils/recover_test.go b/x/utils/recover_test.go index 035b749a..cace7392 100644 --- a/x/utils/recover_test.go +++ b/x/utils/recover_test.go @@ -27,8 +27,8 @@ func TestRecovery(t *testing.T) { // recovery wrapped handler returns error _, err := r.Check(ctx, store, nil, pan) assert.Error(t, err) - assert.Equal(t, "panic: boom", err.Error()) + assert.Equal(t, "boom: panic", err.Error()) _, err = r.Deliver(ctx, store, nil, pan) assert.Error(t, err) - assert.Equal(t, "panic: boom", err.Error()) + assert.Equal(t, "boom: panic", err.Error()) } From 96b1e147150f160111f70b1a49d627c50119ea4c Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 17:05:31 +0100 Subject: [PATCH 12/22] escrow done --- errors/errors.go | 3 ++ x/escrow/errors.go | 76 -------------------------------------------- x/escrow/handler.go | 10 +++--- x/escrow/model.go | 4 +-- x/escrow/msg.go | 6 ++-- x/escrow/msg_test.go | 22 ++++++------- 6 files changed, 24 insertions(+), 97 deletions(-) delete mode 100644 x/escrow/errors.go diff --git a/errors/errors.go b/errors/errors.go index 3aba6202..c2ab5f4a 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -60,6 +60,9 @@ var ( // ErrInvalidInput stands for general input problems indication ErrInvalidInput = Register(14, "invalid input") + // ErrExpired stands for expired entities, normally has to do with block height expirations + ErrExpired = Register(15, "expired") + // ErrPanic is only set when we recover from a panic, so we know to redact potentially sensitive system info ErrPanic = Register(111222, "panic") ) diff --git a/x/escrow/errors.go b/x/escrow/errors.go deleted file mode 100644 index 29aa6f18..00000000 --- a/x/escrow/errors.go +++ /dev/null @@ -1,76 +0,0 @@ -package escrow - -import ( - "fmt" - - "github.com/iov-one/weave/errors" -) - -// ABCI Response Codes -// escrow takes 1010-1020 -const ( - CodeNoEscrow = 1010 - CodeInvalidMetadata = 1013 - CodeInvalidHeight = 1014 - - // CodeInvalidIndex = 1001 - // CodeInvalidWallet = 1002 -) - -var ( - errInvalidMemo = fmt.Errorf("Memo field too long") - errInvalidTimeout = fmt.Errorf("Invalid Timeout") - errInvalidEscrowID = fmt.Errorf("Invalid Escrow ID") - - errNoSuchEscrow = fmt.Errorf("No Escrow with this ID") - - errEscrowExpired = fmt.Errorf("Escrow already expired") - errEscrowNotExpired = fmt.Errorf("Escrow not yet expired") - - // errInvalidIndex = fmt.Errorf("Cannot calculate index") - // errInvalidWalletName = fmt.Errorf("Invalid name for a wallet") - // errChangeWalletName = fmt.Errorf("Wallet already has a name") - // errNoSuchWallet = fmt.Errorf("No wallet exists with this address") -) - -func ErrInvalidCondition(perm []byte) error { - return errors.ErrInvalidInput.Newf("condition: %v", perm) -} -func IsInvalidConditionErr(err error) bool { - return errors.ErrInvalidInput.Is(err) -} - -func ErrInvalidMemo(memo string) error { - return errors.WithLog(memo, errInvalidMemo, CodeInvalidMetadata) -} -func ErrInvalidTimeout(timeout int64) error { - msg := fmt.Sprintf("%d", timeout) - return errors.WithLog(msg, errInvalidTimeout, CodeInvalidMetadata) -} -func ErrInvalidEscrowID(id []byte) error { - msg := "(nil)" - if len(id) > 0 { - msg = fmt.Sprintf("%X", id) - } - return errors.WithLog(msg, errInvalidEscrowID, CodeInvalidMetadata) -} -func IsInvalidMetadataErr(err error) bool { - return errors.HasErrorCode(err, CodeInvalidMetadata) -} - -func ErrNoSuchEscrow(id []byte) error { - msg := fmt.Sprintf("%X", id) - return errors.WithLog(msg, errNoSuchEscrow, CodeNoEscrow) -} -func IsNoSuchEscrowErr(err error) bool { - return errors.HasErrorCode(err, CodeNoEscrow) -} - -func ErrEscrowExpired(timeout int64) error { - msg := fmt.Sprintf("%d", timeout) - return errors.WithLog(msg, errEscrowExpired, CodeInvalidHeight) -} -func ErrEscrowNotExpired(timeout int64) error { - msg := fmt.Sprintf("%d", timeout) - return errors.WithLog(msg, errEscrowNotExpired, CodeInvalidHeight) -} diff --git a/x/escrow/handler.go b/x/escrow/handler.go index 32efe454..273e9214 100644 --- a/x/escrow/handler.go +++ b/x/escrow/handler.go @@ -118,7 +118,7 @@ func (h CreateEscrowHandler) validate(ctx weave.Context, db weave.KVStore, // verify that timeout is in the future height, _ := weave.GetHeight(ctx) if msg.Timeout <= height { - return nil, ErrInvalidTimeout(msg.Timeout) + return nil, errors.ErrInvalidInput.Newf("timeout: %d", msg.Timeout) } // sender must authorize this (if not set, defaults to MainSigner) @@ -228,7 +228,7 @@ func (h ReleaseEscrowHandler) validate(ctx weave.Context, db weave.KVStore, // timeout must not have expired height, _ := weave.GetHeight(ctx) if escrow.Timeout < height { - return nil, nil, ErrEscrowExpired(escrow.Timeout) + return nil, nil, errors.ErrExpired.Newf("escrow %d", escrow.Timeout) } return msg, escrow, nil @@ -306,7 +306,7 @@ func (h ReturnEscrowHandler) validate(ctx weave.Context, db weave.KVStore, // timeout must have expired height, _ := weave.GetHeight(ctx) if height <= escrow.Timeout { - return nil, nil, ErrEscrowNotExpired(escrow.Timeout) + return nil, nil, errors.ErrInvalidState.Newf("escrow not expired %d", escrow.Timeout) } return msg.EscrowId, escrow, nil @@ -393,7 +393,7 @@ func (h UpdateEscrowHandler) validate(ctx weave.Context, db weave.KVStore, // timeout must not have expired height, _ := weave.GetHeight(ctx) if height > escrow.Timeout { - return nil, nil, ErrEscrowExpired(escrow.Timeout) + return nil, nil, errors.ErrExpired.Newf("escrow %d", escrow.Timeout) } // we must have the permission for the items we want to change @@ -427,7 +427,7 @@ func loadEscrow(bucket Bucket, db weave.KVStore, escrowID []byte) (*Escrow, erro } escrow := AsEscrow(obj) if escrow == nil { - return nil, ErrNoSuchEscrow(escrowID) + return nil, errors.ErrEmpty.Newf("escrow %d", escrowID) } return escrow, nil } diff --git a/x/escrow/model.go b/x/escrow/model.go index 8c96df5a..3725f9f0 100644 --- a/x/escrow/model.go +++ b/x/escrow/model.go @@ -30,10 +30,10 @@ func (e *Escrow) Validate() error { return errors.ErrEmpty.New("recipient") } if e.Timeout <= 0 { - return ErrInvalidTimeout(e.Timeout) + return errors.ErrInvalidInput.Newf("timeout: %d", e.Timeout) } if len(e.Memo) > maxMemoSize { - return ErrInvalidMemo(e.Memo) + return errors.ErrInvalidInput.Newf("memo %s", e.Memo) } if err := validateAmount(e.Amount); err != nil { return err diff --git a/x/escrow/msg.go b/x/escrow/msg.go index f3aa59a1..5084931b 100644 --- a/x/escrow/msg.go +++ b/x/escrow/msg.go @@ -66,10 +66,10 @@ func (m *CreateEscrowMsg) Validate() error { return errors.ErrEmpty.New("recipient") } if m.Timeout <= 0 { - return ErrInvalidTimeout(m.Timeout) + return errors.ErrInvalidInput.Newf("timeout: %d", m.Timeout) } if len(m.Memo) > maxMemoSize { - return ErrInvalidMemo(m.Memo) + return errors.ErrInvalidInput.Newf("memo %s", m.Memo) } if err := validateAmount(m.Amount); err != nil { return err @@ -154,7 +154,7 @@ func validateAmount(amount x.Coins) error { func validateEscrowID(id []byte) error { if len(id) != 8 { - return ErrInvalidEscrowID(id) + return errors.ErrInvalidInput.Newf("escrow id: %X", id) } return nil } diff --git a/x/escrow/msg_test.go b/x/escrow/msg_test.go index 452589d9..cab79d79 100644 --- a/x/escrow/msg_test.go +++ b/x/escrow/msg_test.go @@ -77,7 +77,7 @@ func TestCreateEscrowMsg(t *testing.T) { Amount: plus, Timeout: 52, }, - IsInvalidConditionErr, + errors.ErrInvalidInput.Is, }, // negative amount 4: { @@ -117,7 +117,7 @@ func TestCreateEscrowMsg(t *testing.T) { Timeout: 52, Memo: strings.Repeat("foo", 100), }, - IsInvalidMetadataErr, + errors.ErrInvalidInput.Is, }, // invalid timeout 8: { @@ -127,7 +127,7 @@ func TestCreateEscrowMsg(t *testing.T) { Amount: plus, Timeout: -8, }, - IsInvalidMetadataErr, + errors.ErrInvalidInput.Is, }, } @@ -158,7 +158,7 @@ func TestReleaseEscrowMsg(t *testing.T) { check checkErr }{ // nothing - 0: {new(ReleaseEscrowMsg), IsInvalidMetadataErr}, + 0: {new(ReleaseEscrowMsg), errors.ErrInvalidInput.Is}, // proper: valid amount 1: { &ReleaseEscrowMsg{ @@ -179,14 +179,14 @@ func TestReleaseEscrowMsg(t *testing.T) { &ReleaseEscrowMsg{ EscrowId: scarecrow, }, - IsInvalidMetadataErr, + errors.ErrInvalidInput.Is, }, // missing id 4: { &ReleaseEscrowMsg{ Amount: plus, }, - IsInvalidMetadataErr, + errors.ErrInvalidInput.Is, }, // negative amount 5: { @@ -226,7 +226,7 @@ func TestReturnEscrowMsg(t *testing.T) { check checkErr }{ // missing id - 0: {new(ReturnEscrowMsg), IsInvalidMetadataErr}, + 0: {new(ReturnEscrowMsg), errors.ErrInvalidInput.Is}, // proper: valid id 1: { &ReturnEscrowMsg{ @@ -239,7 +239,7 @@ func TestReturnEscrowMsg(t *testing.T) { &ReturnEscrowMsg{ EscrowId: scarecrow, }, - IsInvalidMetadataErr, + errors.ErrInvalidInput.Is, }, } @@ -272,7 +272,7 @@ func TestUpdateEscrowMsg(t *testing.T) { check checkErr }{ // nothing - 0: {new(UpdateEscrowPartiesMsg), IsInvalidMetadataErr}, + 0: {new(UpdateEscrowPartiesMsg), errors.ErrInvalidInput.Is}, // proper: valid id, one valid permission 1: { &UpdateEscrowPartiesMsg{ @@ -294,7 +294,7 @@ func TestUpdateEscrowMsg(t *testing.T) { EscrowId: scarecrow, Sender: a.Address(), }, - IsInvalidMetadataErr, + errors.ErrInvalidInput.Is, }, // allow multiple permissions 4: { @@ -311,7 +311,7 @@ func TestUpdateEscrowMsg(t *testing.T) { EscrowId: escrow, Arbiter: d, }, - IsInvalidConditionErr, + errors.ErrInvalidInput.Is, }, } From 4b5a3f888e731ea522667a68a43092facd82fd0e Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 17:20:45 +0100 Subject: [PATCH 13/22] multisig done --- x/multisig/decorator.go | 2 +- x/multisig/decorator_test.go | 2 +- x/multisig/errors.go | 45 ++---------------------------------- x/multisig/handlers.go | 2 +- x/multisig/handlers_test.go | 10 ++++---- x/multisig/model.go | 6 ++--- x/multisig/msg.go | 17 ++++++++------ 7 files changed, 23 insertions(+), 61 deletions(-) diff --git a/x/multisig/decorator.go b/x/multisig/decorator.go index ce5b8b37..e871d19d 100644 --- a/x/multisig/decorator.go +++ b/x/multisig/decorator.go @@ -86,7 +86,7 @@ func (d Decorator) getContract(store weave.KVStore, id []byte) (*Contract, error } if obj == nil || (obj != nil && obj.Value() == nil) { - return nil, ErrContractNotFound(id) + return nil, errors.ErrNotFound.Newf(contractNotFoundFmt, id) } contract := obj.Value().(*Contract) diff --git a/x/multisig/decorator_test.go b/x/multisig/decorator_test.go index 7292504b..d0de938f 100644 --- a/x/multisig/decorator_test.go +++ b/x/multisig/decorator_test.go @@ -91,7 +91,7 @@ func TestDecorator(t *testing.T) { multisigTx([]byte("foo"), []byte("bad id")), []weave.Condition{a, b}, nil, - ErrContractNotFound([]byte("bad id")), + errors.ErrNotFound.Newf(contractNotFoundFmt, []byte("bad id")), }, // contractID3 is activated by contractID2 { diff --git a/x/multisig/errors.go b/x/multisig/errors.go index 1687c52d..f01b990c 100644 --- a/x/multisig/errors.go +++ b/x/multisig/errors.go @@ -1,47 +1,6 @@ package multisig -import ( - "fmt" - - "github.com/iov-one/weave/errors" -) - -// ABCI Response Codes -// multisig takes 1030-1040 -const ( - CodeInvalidMsg = 1030 - CodeMultisigAuthentication = 1031 -) - var ( - errMissingSigs = fmt.Errorf("Missing sigs") - errInvalidThreshold = fmt.Errorf("Activation threshold must be lower than or equal to the number of sigs") - - errUnauthorizedMultiSig = fmt.Errorf("Multisig authentication failed") - errContractNotFound = fmt.Errorf("Multisig contract not found") + invalidThreshold = "activation/change threshold must be lower than or equal to the number of sigs" + contractNotFoundFmt = "multisig contract not found contract=%X" ) - -func ErrMissingSigs() error { - return errors.WithCode(errMissingSigs, CodeInvalidMsg) -} -func ErrInvalidActivationThreshold() error { - return errors.WithCode(errInvalidThreshold, CodeInvalidMsg) -} -func ErrInvalidChangeThreshold() error { - return errors.WithCode(errInvalidThreshold, CodeInvalidMsg) -} -func IsInvalidMsgErr(err error) bool { - return errors.HasErrorCode(err, CodeInvalidMsg) -} - -func ErrUnauthorizedMultiSig(contract []byte) error { - msg := fmt.Sprintf("contract=%X", contract) - return errors.WithLog(msg, errUnauthorizedMultiSig, CodeMultisigAuthentication) -} -func ErrContractNotFound(contract []byte) error { - msg := fmt.Sprintf("contract=%X", contract) - return errors.WithLog(msg, errContractNotFound, CodeMultisigAuthentication) -} -func IsMultiSigAuthenticationErr(err error) bool { - return errors.HasErrorCode(err, CodeMultisigAuthentication) -} diff --git a/x/multisig/handlers.go b/x/multisig/handlers.go index fa74699b..6a2e1166 100644 --- a/x/multisig/handlers.go +++ b/x/multisig/handlers.go @@ -148,7 +148,7 @@ func (h UpdateContractMsgHandler) validate(ctx weave.Context, db weave.KVStore, return nil, err } if obj == nil || (obj != nil && obj.Value() == nil) { - return nil, ErrContractNotFound(updateContractMsg.Id) + return nil, errors.ErrNotFound.Newf(contractNotFoundFmt, updateContractMsg.Id) } contract := obj.Value().(*Contract) diff --git a/x/multisig/handlers_test.go b/x/multisig/handlers_test.go index 932d1ebe..75edf70a 100644 --- a/x/multisig/handlers_test.go +++ b/x/multisig/handlers_test.go @@ -86,7 +86,7 @@ func TestCreateContractMsgHandler(t *testing.T) { { name: "missing sigs", msg: &CreateContractMsg{}, - err: ErrMissingSigs(), + err: errors.ErrInvalidMsg.New("missing sigs"), }, { name: "bad activation threshold", @@ -95,7 +95,7 @@ func TestCreateContractMsgHandler(t *testing.T) { ActivationThreshold: 4, AdminThreshold: 3, }, - err: ErrInvalidActivationThreshold(), + err: errors.ErrInvalidMsg.New(invalidThreshold), }, { name: "bad admin threshold", @@ -104,7 +104,7 @@ func TestCreateContractMsgHandler(t *testing.T) { ActivationThreshold: 1, AdminThreshold: -1, }, - err: ErrInvalidChangeThreshold(), + err: errors.ErrInvalidMsg.New(invalidThreshold), }, { name: "0 activation threshold", @@ -113,7 +113,7 @@ func TestCreateContractMsgHandler(t *testing.T) { ActivationThreshold: 0, AdminThreshold: 1, }, - err: ErrInvalidChangeThreshold(), + err: errors.ErrInvalidMsg.New(invalidThreshold), }, } @@ -217,7 +217,7 @@ func TestUpdateContractMsgHandler(t *testing.T) { AdminThreshold: 0, }, signers: []weave.Condition{a, b, c, d, e}, - err: ErrInvalidChangeThreshold(), + err: errors.ErrInvalidMsg.New(invalidThreshold), }, } diff --git a/x/multisig/model.go b/x/multisig/model.go index 65b854ad..876be9e8 100644 --- a/x/multisig/model.go +++ b/x/multisig/model.go @@ -19,13 +19,13 @@ var _ orm.CloneableData = (*Contract)(nil) // Validate enforces sigs and threshold boundaries func (c *Contract) Validate() error { if len(c.Sigs) == 0 { - return ErrMissingSigs() + return errors.ErrInvalidMsg.New("missing sigs") } if c.ActivationThreshold <= 0 || int(c.ActivationThreshold) > len(c.Sigs) { - return ErrInvalidActivationThreshold() + return errors.ErrInvalidMsg.New(invalidThreshold) } if c.AdminThreshold <= 0 { - return ErrInvalidChangeThreshold() + return errors.ErrInvalidMsg.New(invalidThreshold) } for _, a := range c.Sigs { if err := weave.Address(a).Validate(); err != nil { diff --git a/x/multisig/msg.go b/x/multisig/msg.go index 415de90e..bd241673 100644 --- a/x/multisig/msg.go +++ b/x/multisig/msg.go @@ -1,6 +1,9 @@ package multisig -import "github.com/iov-one/weave" +import ( + "github.com/iov-one/weave" + "github.com/iov-one/weave/errors" +) const ( pathCreateContractMsg = "multisig/create" @@ -18,13 +21,13 @@ func (CreateContractMsg) Path() string { // Validate enforces sigs and threshold boundaries func (c *CreateContractMsg) Validate() error { if len(c.Sigs) == 0 { - return ErrMissingSigs() + return errors.ErrInvalidMsg.New("missing sigs") } if c.ActivationThreshold <= 0 || int(c.ActivationThreshold) > len(c.Sigs) { - return ErrInvalidActivationThreshold() + return errors.ErrInvalidMsg.New(invalidThreshold) } if c.AdminThreshold <= 0 { - return ErrInvalidChangeThreshold() + return errors.ErrInvalidMsg.New(invalidThreshold) } for _, a := range c.Sigs { if err := weave.Address(a).Validate(); err != nil { @@ -42,13 +45,13 @@ func (UpdateContractMsg) Path() string { // Validate enforces sigs and threshold boundaries func (c *UpdateContractMsg) Validate() error { if len(c.Sigs) == 0 { - return ErrMissingSigs() + return errors.ErrInvalidMsg.New("missing sigs") } if c.ActivationThreshold <= 0 || int(c.ActivationThreshold) > len(c.Sigs) { - return ErrInvalidActivationThreshold() + return errors.ErrInvalidMsg.New(invalidThreshold) } if c.AdminThreshold <= 0 { - return ErrInvalidChangeThreshold() + return errors.ErrInvalidMsg.New(invalidThreshold) } for _, a := range c.Sigs { if err := weave.Address(a).Validate(); err != nil { From 848684e88b147a5c42a0e6949279457e2a222b5c Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 21 Feb 2019 17:34:23 +0100 Subject: [PATCH 14/22] typo fixed --- x/paychan/msg.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/paychan/msg.go b/x/paychan/msg.go index 04844dd4..bd11cbaa 100644 --- a/x/paychan/msg.go +++ b/x/paychan/msg.go @@ -26,10 +26,10 @@ func (m *CreatePaymentChannelMsg) Validate() error { return errors.ErrInvalidMsg.New("missing recipient") } if m.Total == nil || m.Total.IsZero() { - return errors.ErrInvalidMsg.New("inalid total amount") + return errors.ErrInvalidMsg.New("invalid total amount") } if m.Timeout <= 0 { - return errors.ErrInvalidMsg.New("inalid timeout value") + return errors.ErrInvalidMsg.New("invalid timeout value") } if len(m.Memo) > 128 { return errors.ErrInvalidMsg.New("memo too long") From ff2aa5d9f971463deb4bef089ef02b6e8d32fb2f Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 22 Feb 2019 04:29:47 +0100 Subject: [PATCH 15/22] namecoin done --- x/namecoin/errors.go | 58 ++------------------------------------ x/namecoin/errors_test.go | 3 +- x/namecoin/handler.go | 4 +-- x/namecoin/handler_test.go | 14 ++++----- x/namecoin/msg.go | 6 ++-- x/namecoin/token.go | 6 ++-- x/namecoin/wallet.go | 10 +++---- 7 files changed, 24 insertions(+), 77 deletions(-) diff --git a/x/namecoin/errors.go b/x/namecoin/errors.go index 5106b2db..6fa75d84 100644 --- a/x/namecoin/errors.go +++ b/x/namecoin/errors.go @@ -1,60 +1,6 @@ package namecoin -import ( - "fmt" - - "github.com/iov-one/weave/errors" -) - -// ABCI Response Codes -// namecoin takes 1000-1010 -const ( - CodeInvalidToken = 1000 - CodeInvalidIndex = 1001 - CodeInvalidWallet = 1002 -) - var ( - errInvalidTokenName = fmt.Errorf("Invalid token name") - errDuplicateToken = fmt.Errorf("Token with that ticker already exists") - errInvalidSigFigs = fmt.Errorf("Invalid significant figures") - errInvalidIndex = fmt.Errorf("Cannot calculate index") - errInvalidWalletName = fmt.Errorf("Invalid name for a wallet") - errChangeWalletName = fmt.Errorf("Wallet already has a name") - errNoSuchWallet = fmt.Errorf("No wallet exists with this address") + invalidTokenNameFmt = "invalid token name: %s" + invalidSigFigsFmt = "invalid significant figures: %d" ) - -func ErrInvalidTokenName(name string) error { - return errors.WithLog(name, errInvalidTokenName, CodeInvalidToken) -} -func ErrDuplicateToken(name string) error { - return errors.WithLog(name, errDuplicateToken, CodeInvalidToken) -} -func ErrInvalidSigFigs(figs int32) error { - msg := fmt.Sprintf("%d", figs) - return errors.WithLog(msg, errInvalidSigFigs, CodeInvalidToken) -} -func IsInvalidToken(err error) bool { - return errors.HasErrorCode(err, CodeInvalidToken) -} - -func ErrInvalidIndex(reason string) error { - return errors.WithLog(reason, errInvalidIndex, CodeInvalidIndex) -} -func IsInvalidIndex(err error) bool { - return errors.HasErrorCode(err, CodeInvalidIndex) -} - -func ErrChangeWalletName() error { - return errors.WithCode(errChangeWalletName, CodeInvalidWallet) -} -func ErrInvalidWalletName(name string) error { - return errors.WithLog(name, errInvalidWalletName, CodeInvalidWallet) -} -func ErrNoSuchWallet(addr []byte) error { - name := fmt.Sprintf("%X", addr) - return errors.WithLog(name, errNoSuchWallet, CodeInvalidWallet) -} -func IsInvalidWallet(err error) bool { - return errors.HasErrorCode(err, CodeInvalidWallet) -} diff --git a/x/namecoin/errors_test.go b/x/namecoin/errors_test.go index 574bc0db..3e2ea4c0 100644 --- a/x/namecoin/errors_test.go +++ b/x/namecoin/errors_test.go @@ -1,6 +1,7 @@ package namecoin import ( + "github.com/iov-one/weave/errors" "regexp" "testing" @@ -11,6 +12,6 @@ import ( func TestErrNoSuchWallet(t *testing.T) { hasNonASCII := regexp.MustCompile("[[:^ascii:]]").MatchString addr := crypto.GenPrivKeyEd25519().PublicKey().Address() - msg := ErrNoSuchWallet(addr).Error() + msg := errors.ErrNotFound.Newf("wallet %X", addr).Error() require.False(t, hasNonASCII(msg), msg) } diff --git a/x/namecoin/handler.go b/x/namecoin/handler.go index ca3da507..bdcb70bc 100644 --- a/x/namecoin/handler.go +++ b/x/namecoin/handler.go @@ -125,7 +125,7 @@ func (h TokenHandler) validate(ctx weave.Context, db weave.KVStore, return nil, err } if obj != nil { - return nil, ErrDuplicateToken(msg.Ticker) + return nil, errors.ErrDuplicate.Newf("token with ticker %s", msg.Ticker) } return msg, nil @@ -170,7 +170,7 @@ func (h SetNameHandler) Deliver(ctx weave.Context, db weave.KVStore, return res, err } if obj == nil { - return res, ErrNoSuchWallet(msg.Address) + return res, errors.ErrNotFound.Newf("wallet %X", msg.Address) } named := AsNamed(obj) err = named.SetName(msg.Name) diff --git a/x/namecoin/handler_test.go b/x/namecoin/handler_test.go index 5dbe86eb..673b9bc1 100644 --- a/x/namecoin/handler_test.go +++ b/x/namecoin/handler_test.go @@ -137,15 +137,15 @@ func TestNewTokenHandler(t *testing.T) { 1: {nil, nil, nil, BuildTokenMsg("YO", "digga", 7), x.ErrInvalidCurrency.Is, x.ErrInvalidCurrency.Is, "", nil}, 2: {nil, nil, nil, BuildTokenMsg("GOOD", "ill3glz!", 7), - IsInvalidToken, IsInvalidToken, "", nil}, + errors.ErrInvalidInput.Is, errors.ErrInvalidInput.Is, "", nil}, 3: {nil, nil, nil, BuildTokenMsg("GOOD", "my good token", 17), - IsInvalidToken, IsInvalidToken, "", nil}, + errors.ErrInvalidInput.Is, errors.ErrInvalidInput.Is, "", nil}, // valid message, done! 4: {nil, nil, nil, msg, noErr, noErr, ticker, added}, // try to overwrite 5: {nil, nil, []orm.Object{NewToken(ticker, "i was here first", 4)}, msg, - IsInvalidToken, IsInvalidToken, "", nil}, + errors.ErrDuplicate.Is, errors.ErrDuplicate.Is, "", nil}, // different name is fine 6: {nil, nil, []orm.Object{NewToken("OTHR", "i was here first", 4)}, msg, noErr, noErr, ticker, added}, @@ -223,13 +223,13 @@ func TestSetNameHandler(t *testing.T) { 1: {nil, nil, BuildSetNameMsg([]byte{1, 2}, "johnny"), errors.ErrInvalidInput.Is, errors.ErrInvalidInput.Is, nil, nil}, 2: {nil, nil, BuildSetNameMsg(addr, "sh"), - IsInvalidWallet, IsInvalidWallet, nil, nil}, + errors.ErrInvalidInput.Is, errors.ErrInvalidInput.Is, nil, nil}, // no permission to change account 3: {nil, []orm.Object{newUser}, msg, errors.ErrUnauthorized.Is, errors.ErrUnauthorized.Is, nil, nil}, // no account to change - only checked deliver 4: {perm, nil, msg, - noErr, IsInvalidWallet, nil, nil}, + noErr, errors.ErrNotFound.Is, nil, nil}, 5: {perm2, []orm.Object{newUser}, msg, errors.ErrUnauthorized.Is, errors.ErrUnauthorized.Is, nil, nil}, // yes, we changed it! @@ -237,13 +237,13 @@ func TestSetNameHandler(t *testing.T) { noErr, noErr, addr, setUser}, // cannot change already set - only checked deliver? 7: {perm, []orm.Object{setUser}, msg, - noErr, IsInvalidWallet, nil, nil}, + noErr, errors.ErrCannotBeModified.Is, nil, nil}, // cannot create conflict - only checked deliver? 8: {perm, []orm.Object{newUser, dupUser}, msg, noErr, errors.ErrDuplicate.Is, nil, nil}, // cannot change - no such a wallet (should should up by addr2 not addr) 9: {perm, []orm.Object{dupUser}, msg, noErr, - func(err error) bool { return errors.IsSameError(err, ErrNoSuchWallet(addr)) }, + func(err error) bool { return errors.ErrNotFound.Is(err) }, addr, nil}, } diff --git a/x/namecoin/msg.go b/x/namecoin/msg.go index 90065e59..989abaf6 100644 --- a/x/namecoin/msg.go +++ b/x/namecoin/msg.go @@ -40,10 +40,10 @@ func (t *NewTokenMsg) Validate() error { return x.ErrInvalidCurrency.New(t.Ticker) } if !IsTokenName(t.Name) { - return ErrInvalidTokenName(t.Name) + return errors.ErrInvalidInput.Newf(invalidTokenNameFmt, t.Name) } if t.SigFigs < minSigFigs || t.SigFigs > maxSigFigs { - return ErrInvalidSigFigs(t.SigFigs) + return errors.ErrInvalidInput.Newf(invalidSigFigsFmt, t.SigFigs) } return nil } @@ -68,7 +68,7 @@ func (s *SetWalletNameMsg) Validate() error { return errors.ErrInvalidInput.Newf("address: %v", s.Address) } if !IsWalletName(s.Name) { - return ErrInvalidWalletName(s.Name) + return errors.ErrInvalidInput.Newf("wallet name: %v", s.Name) } return nil } diff --git a/x/namecoin/token.go b/x/namecoin/token.go index 7e655852..22f775c2 100644 --- a/x/namecoin/token.go +++ b/x/namecoin/token.go @@ -21,10 +21,10 @@ var _ orm.CloneableData = (*Token)(nil) // Validate ensures the token is valid func (t *Token) Validate() error { if !IsTokenName(t.Name) { - return ErrInvalidTokenName(t.Name) + return errors.ErrInvalidInput.Newf(invalidTokenNameFmt, t.Name) } if t.SigFigs < minSigFigs || t.SigFigs > maxSigFigs { - return ErrInvalidSigFigs(t.SigFigs) + return errors.ErrInvalidInput.Newf(invalidSigFigsFmt, t.SigFigs) } return nil } @@ -101,7 +101,7 @@ func (b TokenBucket) Save(db weave.KVStore, obj orm.Object) error { } name := string(obj.Key()) if !x.IsCC(name) { - return ErrInvalidTokenName(name) + return errors.ErrInvalidInput.Newf(invalidTokenNameFmt, name) } return b.Bucket.Save(db, obj) } diff --git a/x/namecoin/wallet.go b/x/namecoin/wallet.go index af262743..f3e8a019 100644 --- a/x/namecoin/wallet.go +++ b/x/namecoin/wallet.go @@ -31,7 +31,7 @@ func (w *Wallet) SetCoins(coins []*x.Coin) { func (w *Wallet) Validate() error { name := w.GetName() if name != "" && !IsWalletName(name) { - return ErrInvalidWalletName(name) + return errors.ErrInvalidInput.Newf("wallet name: %v", name) } return cash.XCoins(w).Validate() } @@ -47,10 +47,10 @@ func (w *Wallet) Copy() orm.CloneableData { // SetName verifies the name is valid and sets it on the wallet func (w *Wallet) SetName(name string) error { if w.Name != "" { - return ErrChangeWalletName() + return errors.ErrCannotBeModified.New("wallet already has a name") } if !IsWalletName(name) { - return ErrInvalidWalletName(name) + return errors.ErrInvalidInput.Newf("wallet name %s", name) } w.Name = name return nil @@ -147,11 +147,11 @@ func (b WalletBucket) Save(db weave.KVStore, obj orm.Object) error { // simple indexer for Wallet name func nameIndex(obj orm.Object) ([]byte, error) { if obj == nil { - return nil, ErrInvalidIndex("nil") + return nil, errors.ErrInvalidModel.New("nil") } wallet, ok := obj.Value().(*Wallet) if !ok { - return nil, ErrInvalidIndex("Not wallet") + return nil, errors.ErrInvalidModel.New("not wallet") } // big-endian encoded int64 return []byte(wallet.Name), nil From c3f553502c7ad1fa57eeb177c3b433c96e7327af Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 22 Feb 2019 04:46:58 +0100 Subject: [PATCH 16/22] nft done --- cmd/bnsd/x/nft/username/handler.go | 6 +-- cmd/bnsd/x/nft/username/model.go | 10 ++-- cmd/bnsd/x/nft/username/msg.go | 2 +- x/nft/base/handler.go | 6 +-- x/nft/errors.go | 73 +----------------------------- x/nft/errors_test.go | 2 +- x/nft/model.go | 3 +- 7 files changed, 17 insertions(+), 85 deletions(-) diff --git a/cmd/bnsd/x/nft/username/handler.go b/cmd/bnsd/x/nft/username/handler.go index 1ad6f7c5..f60ede20 100644 --- a/cmd/bnsd/x/nft/username/handler.go +++ b/cmd/bnsd/x/nft/username/handler.go @@ -180,7 +180,7 @@ func (h RemoveChainAddressHandler) Deliver(ctx weave.Context, store weave.KVStor return res, errors.ErrUnauthorized } if len(t.GetChainAddresses()) == 0 { - return res, nft.ErrInvalidEntry([]byte("no chain to delete")) + return res, errors.ErrInvalidInput.New("empty chain addresses") } obsoleteAddress := ChainAddress{msg.GetBlockchainID(), msg.GetAddress()} newAddresses := make([]ChainAddress, 0, len(t.GetChainAddresses())) @@ -190,7 +190,7 @@ func (h RemoveChainAddressHandler) Deliver(ctx weave.Context, store weave.KVStor } } if len(newAddresses) == len(t.GetChainAddresses()) { - return res, nft.ErrInvalidEntry([]byte("requested address not registered")) + return res, errors.ErrNotFound.New("requested address not registered") } if err := t.SetChainAddresses(actor, newAddresses); err != nil { return res, err @@ -219,7 +219,7 @@ func loadToken(h tokenHandler, store weave.KVStore, id []byte) (orm.Object, Toke case err != nil: return nil, nil, err case o == nil: - return nil, nil, nft.ErrUnknownID(id) + return nil, nil, errors.ErrNotFound.Newf("username %s", nft.PrintableID(id)) } t, e := AsUsername(o) return o, t, e diff --git a/cmd/bnsd/x/nft/username/model.go b/cmd/bnsd/x/nft/username/model.go index 310e85c5..c41f1789 100644 --- a/cmd/bnsd/x/nft/username/model.go +++ b/cmd/bnsd/x/nft/username/model.go @@ -40,7 +40,7 @@ func (u *UsernameToken) GetChainAddresses() []ChainAddress { func (u *UsernameToken) SetChainAddresses(actor weave.Address, newAddresses []ChainAddress) error { dup := containsDuplicateChains(newAddresses) if dup != nil { - return nft.ErrDuplicateEntry(dup) + return errors.ErrDuplicate.Newf("chain %s", nft.PrintableID(dup)) } u.Details = &TokenDetails{Addresses: newAddresses} return nil @@ -85,7 +85,7 @@ func (t *TokenDetails) Validate() error { } dup := containsDuplicateChains(t.Addresses) if dup != nil { - return nft.ErrDuplicateEntry(dup) + return errors.ErrDuplicate.Newf("chain %s", nft.PrintableID(dup)) } for _, k := range t.Addresses { if err := k.Validate(); err != nil { @@ -113,10 +113,10 @@ func (p ChainAddress) Equals(o ChainAddress) bool { func (p *ChainAddress) Validate() error { if !validBlockchainID(p.BlockchainID) { - return nft.ErrInvalidID(p.BlockchainID) + return errors.ErrInvalidInput.Newf("id: %s", nft.PrintableID(p.BlockchainID)) } if n := len(p.Address); n < 2 || n > 50 { - return nft.ErrInvalidLength() + return errors.ErrInvalidInput.Newf("address length: %s", p.Address) } return nil } @@ -128,7 +128,7 @@ func AsUsername(obj orm.Object) (Token, error) { } x, ok := obj.Value().(*UsernameToken) if !ok { - return nil, nft.ErrUnsupportedTokenType() + return nil, errors.ErrInvalidInput.New(nft.UnsupportedTokenType) } return x, nil } diff --git a/cmd/bnsd/x/nft/username/msg.go b/cmd/bnsd/x/nft/username/msg.go index f9baf557..2614fd91 100644 --- a/cmd/bnsd/x/nft/username/msg.go +++ b/cmd/bnsd/x/nft/username/msg.go @@ -81,7 +81,7 @@ func validateID(id []byte) error { return errors.ErrInternal.New("must not be nil") } if !isValidID(string(id)) { - return nft.ErrInvalidID(id) + return errors.ErrInvalidInput.Newf("id: %s", nft.PrintableID(id)) } return nil } diff --git a/x/nft/base/handler.go b/x/nft/base/handler.go index e4a69a7f..f1038713 100644 --- a/x/nft/base/handler.go +++ b/x/nft/base/handler.go @@ -31,7 +31,7 @@ func asBase(obj orm.Object) (nft.BaseNFT, error) { } x, ok := obj.Value().(nft.BaseNFT) if !ok { - return nil, nft.ErrUnsupportedTokenType() + return nil, errors.ErrInvalidInput.New(nft.UnsupportedTokenType) } return x, nil } @@ -42,7 +42,7 @@ func loadToken(bucket orm.Bucket, store weave.KVStore, id []byte) (orm.Object, n case err != nil: return nil, nil, err case o == nil: - return nil, nil, nft.ErrUnknownID(id) + return nil, nil, errors.ErrNotFound.Newf("nft %s", nft.PrintableID(id)) } t, e := asBase(o) return o, t, e @@ -84,7 +84,7 @@ func (h *ApprovalOpsHandler) Deliver(ctx weave.Context, store weave.KVStore, tx bucket, ok := h.buckets[msg.GetT()] if !ok { - return res, nft.ErrUnsupportedTokenType() + return res, errors.ErrInvalidInput.New(nft.UnsupportedTokenType) } o, t, err := loadToken(bucket, store, msg.GetID()) diff --git a/x/nft/errors.go b/x/nft/errors.go index b0118571..f0900629 100644 --- a/x/nft/errors.go +++ b/x/nft/errors.go @@ -2,85 +2,16 @@ package nft import ( "encoding/hex" - stderrors "errors" - - "github.com/iov-one/weave/errors" ) -// nft and subpackages reserves 500~600 const ( - CodeUnsupportedTokenType uint32 = 500 - CodeInvalidID uint32 = 501 - CodeDuplicateEntry uint32 = 502 - CodeMissingEntry uint32 = 503 - CodeInvalidEntry uint32 = 504 - CodeUnknownID uint32 = 505 - CodeInvalidLength uint32 = 506 - CodeInvalidHost uint32 = 507 - CodeInvalidPort uint32 = 508 - CodeInvalidProtocol uint32 = 509 - CodeInvalidCodec uint32 = 510 - CodeInvalidJson uint32 = 511 -) - -var ( - errUnsupportedTokenType = stderrors.New("Unsupported token type") - errInvalidID = stderrors.New("ID is invalid") - errDuplicateEntry = stderrors.New("Duplicate entry") - errMissingEntry = stderrors.New("Missing entry") - errInvalidEntry = stderrors.New("Invalid entry") - errUnknownID = stderrors.New("Unknown ID") - errInvalidLength = stderrors.New("Invalid length") - errInvalidHost = stderrors.New("Invalid host") - errInvalidPort = stderrors.New("Invalid port") - errInvalidProtocol = stderrors.New("Invalid protocol") - errInvalidCodec = stderrors.New("Invalid codec") - errInvalidJson = stderrors.New("Invalid json") + UnsupportedTokenType = "unsupported token type" ) -// ErrUnsupportedTokenType is when the type passed does not match the expected token type. -func ErrUnsupportedTokenType() error { - return errors.WithCode(errUnsupportedTokenType, CodeUnsupportedTokenType) -} - -func ErrInvalidID(id []byte) error { - return errors.WithLog(printableID(id), errInvalidID, CodeInvalidID) -} -func ErrDuplicateEntry(id []byte) error { - return errors.WithLog(printableID(id), errDuplicateEntry, CodeDuplicateEntry) -} -func ErrMissingEntry() error { - return errors.WithCode(errMissingEntry, CodeMissingEntry) -} -func ErrInvalidEntry(id []byte) error { - return errors.WithLog(printableID(id), errInvalidEntry, CodeInvalidEntry) -} -func ErrUnknownID(id []byte) error { - return errors.WithLog(printableID(id), errUnknownID, CodeUnknownID) -} -func ErrInvalidLength() error { - return errors.WithCode(errInvalidLength, CodeInvalidLength) -} -func ErrInvalidHost() error { - return errors.WithCode(errInvalidHost, CodeInvalidHost) -} -func ErrInvalidPort() error { - return errors.WithCode(errInvalidPort, CodeInvalidPort) -} -func ErrInvalidProtocol() error { - return errors.WithCode(errInvalidProtocol, CodeInvalidProtocol) -} -func ErrInvalidCodec(codec string) error { - return errors.WithLog(codec, errInvalidCodec, CodeInvalidCodec) -} -func ErrInvalidJson() error { - return errors.WithCode(errInvalidJson, CodeInvalidJson) -} - // id's are stored as bytes, but most are ascii text // if in ascii, just convert to string // if not, hex-encode it and prefix with 0x -func printableID(id []byte) string { +func PrintableID(id []byte) string { if len(id) == 0 { return "" } diff --git a/x/nft/errors_test.go b/x/nft/errors_test.go index 96c16e14..f6686676 100644 --- a/x/nft/errors_test.go +++ b/x/nft/errors_test.go @@ -24,7 +24,7 @@ func TestPrintableId(t *testing.T) { for i, tc := range cases { t.Run(fmt.Sprintf("case %d", i), func(tt *testing.T) { - printable := printableID(tc.id) + printable := PrintableID(tc.id) assert.Equal(t, tc.expected, printable) }) } diff --git a/x/nft/model.go b/x/nft/model.go index ee29d24e..8ee8a863 100644 --- a/x/nft/model.go +++ b/x/nft/model.go @@ -2,6 +2,7 @@ package nft import ( "github.com/iov-one/weave" + "github.com/iov-one/weave/errors" "github.com/iov-one/weave/orm" ) @@ -9,7 +10,7 @@ var _ orm.CloneableData = (*NonFungibleToken)(nil) func (m *NonFungibleToken) Validate() error { if !isValidTokenID(m.ID) { - return ErrInvalidID(m.ID) + return errors.ErrInvalidInput.Newf("id: %s", PrintableID(m.ID)) } if err := weave.Address(m.Owner).Validate(); err != nil { From 381dd27460773b2afb6deca287eab17e67535337 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 22 Feb 2019 04:54:09 +0100 Subject: [PATCH 17/22] sigs done --- x/sigs/controller.go | 2 +- x/sigs/controller_test.go | 7 ++++--- x/sigs/errors.go | 29 +---------------------------- x/sigs/model.go | 6 +++--- x/sigs/tx.go | 4 ++-- 5 files changed, 11 insertions(+), 37 deletions(-) diff --git a/x/sigs/controller.go b/x/sigs/controller.go index 9da9de0d..ea374502 100644 --- a/x/sigs/controller.go +++ b/x/sigs/controller.go @@ -105,7 +105,7 @@ the public key signing/verification step */ func BuildSignBytes(signBytes []byte, chainID string, seq int64) ([]byte, error) { if seq < 0 { - return nil, ErrInvalidSequence("negative") + return nil, ErrInvalidSequence.New("negative") } if !weave.IsValidChainID(chainID) { return nil, errors.ErrInvalidInput.Newf("chain id: %v", chainID) diff --git a/x/sigs/controller_test.go b/x/sigs/controller_test.go index 9cc06d7e..17574233 100644 --- a/x/sigs/controller_test.go +++ b/x/sigs/controller_test.go @@ -8,6 +8,7 @@ import ( "github.com/iov-one/weave" "github.com/iov-one/weave/crypto" + "github.com/iov-one/weave/errors" "github.com/iov-one/weave/store" "github.com/iov-one/weave/x" ) @@ -80,7 +81,7 @@ func TestVerifySignature(t *testing.T) { // empty sig _, err = VerifySignature(kv, empty, bz, chainID) assert.Error(t, err) - assert.True(t, IsInvalidSignatureErr(err)) + assert.True(t, errors.ErrUnauthorized.Is(err)) // must start with 0 sign, err := VerifySignature(kv, sig0, bz, chainID) @@ -94,10 +95,10 @@ func TestVerifySignature(t *testing.T) { // jumping and replays are a no-no _, err = VerifySignature(kv, sig1, bz, chainID) assert.Error(t, err) - assert.True(t, IsInvalidSequenceErr(err)) + assert.True(t, ErrInvalidSequence.Is(err)) _, err = VerifySignature(kv, sig13, bz, chainID) assert.Error(t, err) - assert.True(t, IsInvalidSequenceErr(err)) + assert.True(t, ErrInvalidSequence.Is(err)) // different chain doesn't match _, err = VerifySignature(kv, sig2, bz, "metal") diff --git a/x/sigs/errors.go b/x/sigs/errors.go index 9ed3a429..dd9a0654 100644 --- a/x/sigs/errors.go +++ b/x/sigs/errors.go @@ -1,36 +1,9 @@ package sigs import ( - "fmt" - "github.com/iov-one/weave/errors" ) -// ABCI Response Codes -// x/auth reserves 20 ~ 29. -const ( - CodeInvalidSequence uint32 = 20 -) - var ( - errInvalidSequence = fmt.Errorf("Invalid sequence number") + ErrInvalidSequence = errors.Register(120, "invalid sequence number") ) - -func ErrInvalidSequence(why string, args ...interface{}) error { - if len(args) > 0 { - why = fmt.Sprintf(why, args...) - } - return errors.WithLog(why, errInvalidSequence, CodeInvalidSequence) -} -func IsInvalidSequenceErr(err error) bool { - return errors.IsSameError(errInvalidSequence, err) -} - -//------ various invalid signatures ---- -// all will match IsInvalidSignatureError - -func ErrMissingPubkey() error { - return errors.ErrUnauthorized.New("missing public key") -} - -var IsInvalidSignatureErr = errors.ErrUnauthorized.Is diff --git a/x/sigs/model.go b/x/sigs/model.go index f5b6cf21..152ba248 100644 --- a/x/sigs/model.go +++ b/x/sigs/model.go @@ -19,10 +19,10 @@ var _ orm.CloneableData = (*UserData)(nil) func (u *UserData) Validate() error { seq := u.Sequence if seq < 0 { - return ErrInvalidSequence("Seq(%d)", seq) + return ErrInvalidSequence.Newf("Seq(%d)", seq) } if seq > 0 && u.Pubkey == nil { - return ErrInvalidSequence("Seq(%d) needs Pubkey", seq) + return ErrInvalidSequence.Newf("Seq(%d) needs Pubkey", seq) } return nil } @@ -41,7 +41,7 @@ func (u *UserData) Copy() orm.CloneableData { // If not, it will not change the sequence, but return an error func (u *UserData) CheckAndIncrementSequence(check int64) error { if u.Sequence != check { - return ErrInvalidSequence("Mismatch %d != %d", check, u.Sequence) + return ErrInvalidSequence.Newf("Mismatch %d != %d", check, u.Sequence) } u.Sequence++ return nil diff --git a/x/sigs/tx.go b/x/sigs/tx.go index e94d903d..fd70982c 100644 --- a/x/sigs/tx.go +++ b/x/sigs/tx.go @@ -22,10 +22,10 @@ type SignedTx interface { func (s *StdSignature) Validate() error { seq := s.GetSequence() if seq < 0 { - return ErrInvalidSequence("Negative") + return ErrInvalidSequence.New("Negative") } if s.Pubkey == nil { - return ErrMissingPubkey() + return errors.ErrUnauthorized.New("missing public key") } if s.Signature == nil { return errors.ErrUnauthorized.New("missing signature") From 4219ee5f0a8fb3c511514261dfd9cee40682f1b4 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 22 Feb 2019 04:58:17 +0100 Subject: [PATCH 18/22] more error refactoring --- abci_test.go | 1 - app/router.go | 20 ++------------------ app/router_test.go | 7 ++++--- errors/common.go | 9 --------- errors/main.go | 26 -------------------------- 5 files changed, 6 insertions(+), 57 deletions(-) diff --git a/abci_test.go b/abci_test.go index 98ac8486..d1b68e2e 100644 --- a/abci_test.go +++ b/abci_test.go @@ -20,7 +20,6 @@ func TestCreateErrorResult(t *testing.T) { {fmt.Errorf("base"), "base", errors.ErrInternal.ABCICode()}, {pkerr.New("dave"), "dave", errors.ErrInternal.ABCICode()}, {errors.Wrap(fmt.Errorf("demo"), "wrapped"), "wrapped: demo", errors.ErrInternal.ABCICode()}, - {errors.WithCode(fmt.Errorf("no sender"), 11), "no sender", 11}, {errors.ErrInvalidInput.New("unable to decode"), errors.ErrInvalidInput.New("unable to decode").Error(), errors.ErrInvalidInput.ABCICode()}, } diff --git a/app/router.go b/app/router.go index 9f5e81e2..613b673d 100644 --- a/app/router.go +++ b/app/router.go @@ -14,22 +14,6 @@ const DefaultRouterSize = 10 // isPath is the RegExp to ensure the routes make sense var isPath = regexp.MustCompile(`^[a-zA-Z0-9_/]+$`).MatchString -// CodeNoSuchPath is an ABCI Response Codes -// Base SDK reserves 0 ~ 99. App uses 10 ~ 19 -const CodeNoSuchPath uint32 = 10 - -var errNoSuchPath = fmt.Errorf("Path not registered") - -// ErrNoSuchPath constructs an error when router doesn't know the path -func ErrNoSuchPath(path string) error { - return errors.WithLog(path, errNoSuchPath, CodeNoSuchPath) -} - -// IsNoSuchPathErr checks if this is an unknown route error -func IsNoSuchPathErr(err error) bool { - return errors.IsSameError(errNoSuchPath, err) -} - // Router allows us to register many handlers with different // paths and then direct each message to the proper handler. // @@ -114,12 +98,12 @@ var _ weave.Handler = noSuchPathHandler{} func (h noSuchPathHandler) Check(ctx weave.Context, store weave.KVStore, tx weave.Tx) (weave.CheckResult, error) { - return weave.CheckResult{}, ErrNoSuchPath(h.path) + return weave.CheckResult{}, errors.ErrNotFound.Newf("path: %s", h.path) } // Deliver always returns ErrNoSuchPath func (h noSuchPathHandler) Deliver(ctx weave.Context, store weave.KVStore, tx weave.Tx) (weave.DeliverResult, error) { - return weave.DeliverResult{}, ErrNoSuchPath(h.path) + return weave.DeliverResult{}, errors.ErrNotFound.Newf("path: %s", h.path) } diff --git a/app/router_test.go b/app/router_test.go index 508d0251..8a737f5f 100644 --- a/app/router_test.go +++ b/app/router_test.go @@ -2,6 +2,7 @@ package app import ( "fmt" + "github.com/iov-one/weave/errors" "testing" "github.com/stretchr/testify/assert" @@ -37,16 +38,16 @@ func TestRouter(t *testing.T) { // check errors handler is also looked up _, err = r.Handler(bad).Deliver(nil, nil, nil) assert.Error(t, err) - assert.False(t, IsNoSuchPathErr(err)) + assert.False(t, errors.ErrNotFound.Is(err)) assert.Equal(t, msg, err.Error()) assert.Equal(t, 2, counter.GetCount()) // make sure not found returns an error handler as well _, err = r.Handler(missing).Deliver(nil, nil, nil) assert.Error(t, err) - assert.True(t, IsNoSuchPathErr(err)) + assert.True(t, errors.ErrNotFound.Is(err)) _, err = r.Handler(missing).Check(nil, nil, nil) assert.Error(t, err) - assert.True(t, IsNoSuchPathErr(err)) + assert.True(t, errors.ErrNotFound.Is(err)) assert.Equal(t, 2, counter.GetCount()) } diff --git a/errors/common.go b/errors/common.go index 38ed182b..ce822965 100644 --- a/errors/common.go +++ b/errors/common.go @@ -2,17 +2,8 @@ package errors import ( "fmt" - - "github.com/pkg/errors" ) -// IsSameError returns true if these errors have the same root cause. -// pattern is the expected error type and should always be non-nil -// err may be anything and returns true if it is a wrapped version of pattern -func IsSameError(pattern error, err error) bool { - return err != nil && (errors.Cause(err) == errors.Cause(pattern)) -} - // HasErrorCode checks if this error would return the named error code func HasErrorCode(err error, code uint32) bool { if tm, ok := err.(coder); ok { diff --git a/errors/main.go b/errors/main.go index 015ad4d3..b1fb3214 100644 --- a/errors/main.go +++ b/errors/main.go @@ -16,32 +16,6 @@ type TMError interface { ABCILog() string } -// WithCode adds a stacktrace if necessary and sets the code and msg, -// overriding the code if err was already TMError -func WithCode(err error, code uint32) TMError { - // add a stack only if not present - st, ok := err.(stackTracer) - if !ok { - st = errors.WithStack(err).(stackTracer) - } - // TODO: preserve log better??? - // and then wrap it with TMError info - return tmerror{ - stackTracer: st, - code: code, - log: err.Error(), - } -} - -// WithLog prepends some text to the error, then calls WithCode -// It wraps the original error, so IsSameError will still match on err -// -// Since -func WithLog(prefix string, err error, code uint32) TMError { - e2 := errors.WithMessage(err, prefix) - return WithCode(e2, code) -} - ////////////////////////////////////////////////// // tmerror is generic implementation of TMError From dd3080e99c4d9f3d58b12927248e66a9d1916b86 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 22 Feb 2019 05:07:23 +0100 Subject: [PATCH 19/22] doc --- errors/doc.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 errors/doc.go diff --git a/errors/doc.go b/errors/doc.go new file mode 100644 index 00000000..2e936787 --- /dev/null +++ b/errors/doc.go @@ -0,0 +1,17 @@ +/** + Package errors implements custom error interfaces for weave. + + The idea is to reuse as many errors from this package as possible and define custom package + errors when absolutely necessary. It is best to define a new error here if you feel it's going to + be somewhat package-agnostic. + + x/multisig is a good package to take a look at in terms of usage with predefined strings with/without formatting. + x/validators and x/sigs define some custom errors. + + If you want to register a custom error - use Register(code, description). + For reusing errors - use Errxxx.New and Errxxx.Newf. + + Also, error package defines a convenient Is helper to compare errors as well as each err defines an Is + helper to compare errors directly to that type. + */ +package errors From 8a4a5763b17603eb5a9ababb3af20f90146a1074 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 22 Feb 2019 05:08:05 +0100 Subject: [PATCH 20/22] doc update --- errors/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors/doc.go b/errors/doc.go index 2e936787..6515d657 100644 --- a/errors/doc.go +++ b/errors/doc.go @@ -11,7 +11,7 @@ If you want to register a custom error - use Register(code, description). For reusing errors - use Errxxx.New and Errxxx.Newf. - Also, error package defines a convenient Is helper to compare errors as well as each err defines an Is + Also, error package defines a convenient Is helper to compare errors, also each Error defines an Is helper to compare errors directly to that type. */ package errors From 8f34244773c198723baac9ff64369a19424cbf8c Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 22 Feb 2019 17:12:12 +0100 Subject: [PATCH 21/22] fix review comments --- cmd/bnsd/x/nft/username/handler.go | 8 ++++---- cmd/bnsd/x/nft/username/model.go | 2 +- errors/doc.go | 2 ++ x/batch/msg.go | 5 +++-- x/distribution/handler_test.go | 2 +- x/namecoin/errors_test.go | 2 +- x/namecoin/handler.go | 2 +- 7 files changed, 13 insertions(+), 10 deletions(-) diff --git a/cmd/bnsd/x/nft/username/handler.go b/cmd/bnsd/x/nft/username/handler.go index f60ede20..692c8b16 100644 --- a/cmd/bnsd/x/nft/username/handler.go +++ b/cmd/bnsd/x/nft/username/handler.go @@ -82,11 +82,11 @@ func (h IssueHandler) validate(ctx weave.Context, tx weave.Tx) (*IssueTokenMsg, // check permissions if h.issuer != nil { if !h.auth.HasAddress(ctx, h.issuer) { - return nil, errors.ErrUnauthorized + return nil, errors.ErrUnauthorized.New("") } } else { if !h.auth.HasAddress(ctx, msg.Owner) { - return nil, errors.ErrUnauthorized + return nil, errors.ErrUnauthorized.New("") } } return msg, nil @@ -122,7 +122,7 @@ func (h AddChainAddressHandler) Deliver(ctx weave.Context, store weave.KVStore, actor := nft.FindActor(h.auth, ctx, t, nft.UpdateDetails) if actor == nil { - return res, errors.ErrUnauthorized + return res, errors.ErrUnauthorized.New("") } allKeys := append(t.GetChainAddresses(), ChainAddress{msg.GetBlockchainID(), msg.GetAddress()}) if err := t.SetChainAddresses(actor, allKeys); err != nil { @@ -177,7 +177,7 @@ func (h RemoveChainAddressHandler) Deliver(ctx weave.Context, store weave.KVStor actor := nft.FindActor(h.auth, ctx, t, nft.UpdateDetails) if actor == nil { - return res, errors.ErrUnauthorized + return res, errors.ErrUnauthorized.New("") } if len(t.GetChainAddresses()) == 0 { return res, errors.ErrInvalidInput.New("empty chain addresses") diff --git a/cmd/bnsd/x/nft/username/model.go b/cmd/bnsd/x/nft/username/model.go index c41f1789..049c1e7c 100644 --- a/cmd/bnsd/x/nft/username/model.go +++ b/cmd/bnsd/x/nft/username/model.go @@ -81,7 +81,7 @@ func (t *TokenDetails) Clone() *TokenDetails { func (t *TokenDetails) Validate() error { if t == nil { - return errors.ErrInternal.New("must not be nil") + return errors.ErrInternal.New("token details must not be nil") } dup := containsDuplicateChains(t.Addresses) if dup != nil { diff --git a/errors/doc.go b/errors/doc.go index 6515d657..b211a58c 100644 --- a/errors/doc.go +++ b/errors/doc.go @@ -10,6 +10,8 @@ If you want to register a custom error - use Register(code, description). For reusing errors - use Errxxx.New and Errxxx.Newf. + Code stands for ABCI error code, which allows to distinguish types of errors + on the client side and act accordingly. Also, error package defines a convenient Is helper to compare errors, also each Error defines an Is helper to compare errors directly to that type. diff --git a/x/batch/msg.go b/x/batch/msg.go index c052f876..189cc057 100644 --- a/x/batch/msg.go +++ b/x/batch/msg.go @@ -22,8 +22,9 @@ func Validate(msg Msg) error { return errors.Wrap(err, "cannot retrieve batch message") } - if len(l) > MaxBatchMessages { - return errors.ErrInvalidInput.New("transaction is too large") + msgNum := len(l) + if msgNum > MaxBatchMessages { + return errors.ErrInvalidInput.New("transaction is too large, max: %d vs current: %d", MaxBatchMessages, msgNum) } return nil } diff --git a/x/distribution/handler_test.go b/x/distribution/handler_test.go index e66234b9..1e809c4e 100644 --- a/x/distribution/handler_test.go +++ b/x/distribution/handler_test.go @@ -320,7 +320,7 @@ func TestHandlers(t *testing.T) { for _, a := range tc.prepareAccounts { for _, c := range a.coins { if err := ctrl.IssueCoins(db, a.address, *c); err != nil { - t.Fatalf("cannot issue %q to %x: %s", c, a.address, err) + t.Fatalf("cannot issue %q to %s: %s", c, a.address, err) } } } diff --git a/x/namecoin/errors_test.go b/x/namecoin/errors_test.go index 3e2ea4c0..268f0b59 100644 --- a/x/namecoin/errors_test.go +++ b/x/namecoin/errors_test.go @@ -12,6 +12,6 @@ import ( func TestErrNoSuchWallet(t *testing.T) { hasNonASCII := regexp.MustCompile("[[:^ascii:]]").MatchString addr := crypto.GenPrivKeyEd25519().PublicKey().Address() - msg := errors.ErrNotFound.Newf("wallet %X", addr).Error() + msg := errors.ErrNotFound.Newf("wallet %s", addr).Error() require.False(t, hasNonASCII(msg), msg) } diff --git a/x/namecoin/handler.go b/x/namecoin/handler.go index bdcb70bc..62ea446f 100644 --- a/x/namecoin/handler.go +++ b/x/namecoin/handler.go @@ -170,7 +170,7 @@ func (h SetNameHandler) Deliver(ctx weave.Context, db weave.KVStore, return res, err } if obj == nil { - return res, errors.ErrNotFound.Newf("wallet %X", msg.Address) + return res, errors.ErrNotFound.Newf("wallet %s", msg.Address) } named := AsNamed(obj) err = named.SetName(msg.Name) From 151e6a1973557deee725cab0b0516f296cc56870 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 22 Feb 2019 17:18:59 +0100 Subject: [PATCH 22/22] newf --- x/batch/msg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/batch/msg.go b/x/batch/msg.go index 189cc057..0673e5b6 100644 --- a/x/batch/msg.go +++ b/x/batch/msg.go @@ -24,7 +24,7 @@ func Validate(msg Msg) error { msgNum := len(l) if msgNum > MaxBatchMessages { - return errors.ErrInvalidInput.New("transaction is too large, max: %d vs current: %d", MaxBatchMessages, msgNum) + return errors.ErrInvalidInput.Newf("transaction is too large, max: %d vs current: %d", MaxBatchMessages, msgNum) } return nil }