Improve code coverage for UT and fixed bug
Thanks @cschuchardt88 , I'll keep adding UT
@chenzhitong
If you could please update the description of the 1st comment of what this PR covers. So when we merge, we will know.
I need some time to ensure that your update will not change existing behavior. for instance, if the original code should throw exception, it should not return null in your code.
Base58CheckDecode's check may cause hard fork, we'd better have an empty string tested here on both older version and newer version
Base58CheckDecode's check may cause hard fork, we'd better have an empty string tested here on both older version (line 39) and newer version
In both older and newer versions, Base58CheckDecode throws an exception when an empty string is entered.
In the old version, if(input is null) was always false, because the parameter type is string, not string?
In the old version,
if(input is null)was alwaysfalse, because the parameter type isstring, notstring?
I thought that the empty string "" may cause differ results in the two versions. In previous version it will just return empty byte array new byte[]{} while it will throw exception in this new version?
Seems that the empty string will throw FormatException anyway. Sorry. Ignore my comment above. Let's go on.
One more concern is that I'm not sure that if the FormatException and ArgumentNullException could be detected by VM scripts. If there is a way for detecting such exceptions, maybe there still exists hardfork risks. Could you please take a look? @Jim8y
Yes it will be detected by the ApplicationEngine.
Just in case, the exception type will not be modified here
Not ready until changes are made.
@Jim8y do you want to check the last changes?