Fix encoding user input in failed transaction message #1376
Conversation
…-input-in-failed-transaction-message
m4ksio
requested review from
huitseeker,
simonhf,
ramtinms and
janezpodhostnik
m4ksio
changed the title
m4ksio
changed the title
| @@ -262,7 +263,8 @@ func (e *AccountKeyHandler) AddEncodedAccountKey(address runtime.Address, encode | |||
|
|
|||
| publicKey, err = flow.DecodeRuntimeAccountPublicKey(encodedPublicKey, 0) | |||
| if err != nil { | |||
| err = errors.NewValueErrorf(string(encodedPublicKey), "invalid encoded public key value: %w", err) | |||
| hexEncodedPublicKey := hex.EncodeToString(encodedPublicKey) | |||
| err = errors.NewValueErrorf(hexEncodedPublicKey, "invalid encoded public key value: %w", err) | |||
There was a problem hiding this comment.
nit: In so far as we're using a NewValueErrorf already, could we just interpolate with "%x"?
…ransaction-message
Codecov Report
@@ Coverage Diff @@
## master #1376 +/- ##
==========================================
- Coverage 55.39% 55.37% -0.03%
==========================================
Files 510 510
Lines 31857 31858 +1
==========================================
- Hits 17648 17640 -8
- Misses 11834 11842 +8
- Partials 2375 2376 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Looks good to me. Though, I don't think I have enough background to reliably find subtle edge cases.
Style suggestion:
would be great to document in the godoc of method
flow-go/fvm/handler/accountKey.go
Line 250 in 71c36cf
what specific error types it returns under which conditions (at least
AccountNotFoundError and ValueError)
| require.NoError(t, err) | ||
|
|
||
| err = akh.AddEncodedAccountKey(runtime.Address(address), encodedPublicKey) | ||
| require.Error(t, err) |
There was a problem hiding this comment.
lets check the specific error type here, you are expecting to return
…n-failed-transaction-message' into m4ksio/fix-encoidng-user-input-in-failed-transaction-message
Add extra check for error type in a test
…ransaction-message
…ransaction-message
…ransaction-message
|
bors merge |
bors
bot
deleted the
m4ksio/fix-encoidng-user-input-in-failed-transaction-message
branch
No description provided.