ref/haskell: refactor error handling for bech32Decode #41
Conversation
manifoldhiker
changed the title
manifoldhiker
force-pushed
the
mitutee/ltb-7-refactor-error-handling
branch
from
8ddde22 to
1327235
Compare
| maybeToRight :: l -> Maybe r -> Either l r | ||
| maybeToRight l = maybe (Left l) Right | ||
|
|
||
| verify :: Bool -> a -> Either a () |
There was a problem hiding this comment.
I feel that this would work better with the argument order reversed. Then it would be similar to maybeToRight.
| @@ -75,19 +77,19 @@ tests :: TestTree | |||
| tests = testGroup "Tests" | |||
| [ testCase "Checksums" $ forM_ validChecksums $ \checksum -> do | |||
| case bech32Decode checksum of | |||
| Nothing -> assertFailure (show checksum) | |||
| Just (resultHRP, resultData) -> do | |||
| Left err -> assertFailure (show checksum ++ show err) | |||
There was a problem hiding this comment.
something like intercalate ", " [show checksum, show err] would stop the assert failure message from being smooshed together.
| @@ -156,4 +177,14 @@ segwitDecode hrp addr = do | |||
| segwitEncode :: HRP -> Word8 -> Data -> Maybe BS.ByteString | |||
| segwitEncode hrp witver witprog = do | |||
| guard $ segwitCheck witver witprog | |||
| bech32Encode hrp $ UnsafeWord5 witver : toBase32 witprog | |||
| rightToMaybe $ bech32Encode hrp $ UnsafeWord5 witver : toBase32 witprog | |||
There was a problem hiding this comment.
I think foo . bar $ x is preferred to foo $ bar $ x generally speaking. But this isn't show-stopping.
| -- test that a corrupted checksum fails decoding. | ||
| let (hrp, rest) = BSC.breakEnd (== '1') checksum | ||
| Just (first, rest') = BS.uncons rest | ||
| checksumCorrupted = (hrp `BS.snoc` (first `xor` 1)) `BS.append` rest' | ||
| assertBool (show checksum ++ " corrupted") $ isNothing (bech32Decode checksumCorrupted) | ||
| assertBool (show checksum ++ " corrupted") $ isLeft (bech32Decode checksumCorrupted) |
There was a problem hiding this comment.
Shouldn't this be isError ChecksumVerificationFail instead of isLeft?
There was a problem hiding this comment.
Okay, the choice of corruption sometimes produces invalid characters.
| assertEqual (show checksum ++ " re-encode") expectedChecksum checksumEncoded | ||
| , testCase "Invalid checksums" $ forM_ invalidChecksums $ | ||
| \checksum -> assertBool (show checksum) (isNothing $ bech32Decode checksum) | ||
| \checksum -> assertBool (show checksum) (isLeft $ bech32Decode checksum) |
There was a problem hiding this comment.
Same here: Shouldn't this be isError ChecksumVerificationFail instead of isLeft?
There was a problem hiding this comment.
Okay I see these fail for various different reasons.
The main reason to refactor was the
Maybein a signature. It is not clear why the given bech32 string is invalid.