web3@1.0.0-beta.34 - personal_sign puts callback in json rpc params

[kumavis]

web3.personal.sign puts callback in json rpc params, which is invalid json

bug reproduction steps are here
https://github.com/kumavis/web3-beta48-bug

requires web3 browser like metamask or similar

includes a hosted demo so you dont need to build anything

[kumavis]

sounds like this issue affects other methods in addition to personal_sign
see MetaMask/metamask-extension#6080

[kumavis]

conclusion from MetaMask/metamask-extension#6262 (comment):

for web3@1.0.0-beta.38 the web3.eth.personal.sign requires a 3rd parameter, a password string (unused in metamask)

if you were using a callback before, it accidently includes the callback in the params in place of the password string. the callback is not json serializeable, thus throwing the error.

[nivida]

MetaMask/metamask-extension#6080

This issue is a support issue and not a web3 bug.

MetaMask/metamask-extension#6262 (comment)

There was an issue with the provider which thrown this error but this got fixed with beta.48.

for web3@1.0.0-beta.38 the web3.eth.personal.sign requires a 3rd parameter, a password string (unused in metamask)

The personal sign JSON-RPC documentation defines a third parameter password. The third parameter was never optional as you can see in the documentation of web3.

[kumavis]

Seems like the documentation is out of line with real world usage and the behavior that web3js previously supported.

[kumavis]

the crux of MetaMask/metamask-extension#6080 is that web3js is not validating its params to be JSON serializeable. it just happens to explode in a non-obvious and metamask specific way after breaking changes in web3js.

[kumavis]

also - this is in no way a "support" issue. Either close as "wont fix" or consider reversing the breaking change and fixing the non-matching documentation.

[nivida]

It was a bug and not a feature. The documentation was added two years ago and it was not thought to be optional. It's also not documented as optional in the geth and parity documentation.

also - this is in no way a "support" issue. Either close as "wont fix" or consider reversing the breaking change and fixing the non-matching documentation.

It is a support issue because the documentation was always correct.