Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bech32 character mutation test occasionally fails. #331

Closed
2 tasks done
jonathanknowles opened this issue May 28, 2019 · 3 comments
Closed
2 tasks done

Bech32 character mutation test occasionally fails. #331

jonathanknowles opened this issue May 28, 2019 · 3 comments
Assignees

Comments

@jonathanknowles
Copy link
Member

jonathanknowles commented May 28, 2019

Release Operating System Cause
next Windows & OSX & Linux) Code

Context

https://travis-ci.org/input-output-hk/cardano-wallet/jobs/538075040

Steps to Reproduce

Run the test suite with --match "mutated", and with seed 998627727.

Expected behavior

The test suite will pass.

Actual behavior

We see the following error:

      test/Codec/Binary/Bech32Spec.hs:185:9: 
      1) Codec.Binary.Bech32, Decoding a corrupted string should fail, Decoding fails when a single character is mutated.
           *** Failed! (after 36 tests and 1 shrink):
           expected: Left StringToDecodeMissingSeparatorChar
            but got: Left (StringToDecodeContainsInvalidChars [CharPosition 2]), expected: Left (StringToDecodeContainsInvalidChars [])
            but got: Left (StringToDecodeContainsInvalidChars [CharPosition 2]), expected: Left (StringToDecodeContainsInvalidChars [CharPosition 5])
            but got: Left (StringToDecodeContainsInvalidChars [CharPosition 2])
           ValidBech32String {getValidBech32String = "h1?ac10m7flkn0aked", humanReadablePart = HumanReadablePart "h1?ac", unencodedDataPart = DataPart "0m7flk"}
           DataChar {getDataChar = 'q'}
           index of mutated char: 5
                   original char: '1'
                replacement char: 'q'
                 original string: "h1?ac10m7flkn0aked"
                corrupted string: "h1?acq0m7flkn0aked"

Superficially, this appears to be related to the location detection code.

Resolution Plan

  • Figure out why this particular example is failing.
  • Either fix the test case (if our expectations were invalid), or fix the code (if it's doing the wrong thing).

PR

Number Base
#332 master

QA

  • Mutation now only occurs in the data part to avoid accidentally mutating the separator. This still checks the main properties we want for the bech32 encoding since, errors in the human-readable part are very unlikely (would still be caught though, but it's harder to instrument in the test suite).

Cf: https://github.com/input-output-hk/cardano-wallet/blob/master/lib/bech32/test/Codec/Binary/Bech32Spec.hs#L127-L132

@jonathanknowles
Copy link
Member Author

jonathanknowles commented May 28, 2019

This particular failure appears to occur because the separator character was replaced by a different character. This then causes the decoder to treat the earlier 1 as a separator character. The decoder correctly rejects the new string, but with an error that was different from what the test case was expecting.

Recommendation: change the test case to make it just check for isLeft, for the moment.

At a later date, we can do something cleverer, by treating the following mutations differently:

  1. a mutation of the HRP
  2. a mutation of the data part
  3. a mutation of the separator

(Or we might decide that the location detection logic is too much trouble, and just go for a simpler set of errors.)

@jonathanknowles
Copy link
Member Author

Fix available here: #332

@piotr-iohk
Copy link
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants