search

neo-project / neo

NEO Smart Economy
MIT License
3.47k stars 1.03k forks source link

Loading invalid wallet JSON fails w/o providing details about the problem #2474

Open devhawk opened 3 years ago

devhawk commented 3 years ago

tried to load a wallet json file where the isDefault property name was incorrectly cased as isdefault. Code threw a generic NullReferenceException which I could only figure out by running the code under a debugger and stepping thru NEP6Account.FromJson.

NEP6Account.FromJson should be throwing a FormatException + details on the specific issue (in this case, something like "isDefault boolean property missing")

System.NullReferenceException: Object reference not set to an instance of an object.
   at Neo.Wallets.NEP6.NEP6Account.FromJson(JObject json, NEP6Wallet wallet) in c:\Users\harry\Source\neo\official\core\src\neo\Wallets\NEP6\NEP6Account.cs:line 33
   at Neo.Wallets.NEP6.NEP6Wallet.<LoadFromJson>b__12_0(JObject p) in c:\Users\harry\Source\neo\official\core\src\neo\Wallets\NEP6\NEP6Wallet.cs:line 77
   at System.Linq.Enumerable.SelectIListIterator`2.MoveNext()
   at System.Linq.Enumerable.ToDictionary[TSource,TKey](IEnumerable`1 source, Func`2 keySelector, IEqualityComparer`1 comparer)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey](IEnumerable`1 source, Func`2 keySelector)
   at Neo.Wallets.NEP6.NEP6Wallet.LoadFromJson(JObject wallet, ScryptParameters& scrypt, Dictionary`2& accounts, JObject& extra) in c:\Users\harry\Source\neo\official\core\src\neo\Wallets\NEP6\NEP6Wallet.cs:line 77
   at Neo.Wallets.NEP6.NEP6Wallet..ctor(String path, ProtocolSettings settings, String name) in c:\Users\harry\Source\neo\official\core\src\neo\Wallets\NEP6\NEP6Wallet.cs:line 49
   at deploy_tool.Program.OnExecuteAsync(IFileSystem fileSystem, IConsole console) in C:\Users\harry\Source\neo\seattle\playground\deploy-tool\Program.cs:line 73
erikzhang commented 3 years ago

According to NEP-6, it should indeed be isDefault. This issue has been fixed in #2451.

ixje commented 3 years ago

How about updating the NEP instead of the code? As far as I could tell all json output in the code base is lower case (e.g. Block, Transaction, Manifest etc), there was even a special issue for it on the modules repo: https://github.com/neo-project/neo-modules/pull/277

Now isDefault is the only property that violates this.

devhawk commented 3 years ago

According to NEP-6, it should indeed be isDefault. This issue has been fixed in #2451.

Yes, I indicated that in isDefault was incorrectly cased. This issue is to track fixing the exception message that is generated when the format is wrong.