Conversation
danforbes
force-pushed
the
feat/block/take-hex
branch
from
bd151b7 to
afd13b9
Compare
amaury1093
left a comment
amaury1093
left a comment
There was a problem hiding this comment.
lgtm, would just add the 32-byte check
danforbes
force-pushed
the
feat/block/take-hex
branch
3 times, most recently
from
5b6ed12 to
ffb2e96
Compare
|
Converting this to draft so I can rebase on top of the ESLint changes 🚀 |
danforbes
force-pushed
the
feat/block/take-hex
branch
from
ffb2e96 to
3bdebdf
Compare
|
Okay rebased and linted 👍 |
src/main.ts
Outdated
| return api.createType('BlockHash', blockId); | ||
| } else if (isHexStr) { | ||
| throw { | ||
| error: `Cannot get block hash for ${blockId}. Hex string block IDs must be 66-characters (32-bytes) in length.`, |
There was a problem hiding this comment.
I think it'd be good to convert this to a number if some people's systems use hex. There's no way block number collides with a hash that's 32 bytes.
There was a problem hiding this comment.
Not sure I totally understand, but would the conversion be surfacing an additional point of failure/ misdirection that could obscure the original error? i.e if someone accidentally sends the string "Hello", but we convert it to a number before interpolating into the error message, it could throw off debugging.
There was a problem hiding this comment.
Yeah, I think I'm with Zeke on this one but I'm still not completely sure this is what Joe was suggesting so I'll just hold off.
danforbes
force-pushed
the
feat/block/take-hex
branch
from
835aad8 to
f1b656a
Compare
danforbes
force-pushed
the
feat/block/take-hex
branch
from
f1b656a to
75934cc
Compare
joepetrowski
left a comment
joepetrowski
left a comment
There was a problem hiding this comment.
This I can get behind, much clearer about what is an acceptable input and much more useful than before. 🎉
4 participants
Closes #76