Request: ensure '0x' prefixing for all data fields

[asmello]

Ok, so this started as an issue I filled to Parity, but it turns out the fix should be made in web3.js instead.

There's a very subtle and hard to trace bug caused (for example) when you submit the contract bytecode data without the 0x prefix. Sample code:

let web3 = new Web3(new Web3.providers.HttpProvider("http://localhost:8545"));

let compiledCode = solc.compile(code);
let abiDefinition = JSON.parse(compiledCode.contracts[':Voting'].interface);
let VotingContract = web3.eth.contract(abiDefinition);
let byteCode = compiledCode.contracts[':Voting'].bytecode;

let deployedContract = VotingContract.new(['Rama','Nick','Jose'],{data: byteCode, from: web3.eth.accounts[0], gas: 4700000});

The last line fails because on the line before solc yields hex-encoded data without the 0x prefix. Both Geth and TestingRPC implementations are flexible enough to accept that payload as is, even though it doesn't strictly conform to the standard, but when I tested this code against a Parity backend I got a very unhelpful invalid params exception.

An argument could be made to change the solc implementation to return an 0x-prefixed string, but I think it would be best to add some code to the web3.js implementation to automatically do that prefixing when required. That is already the case for parity.js.

Note that nowhere in the JS API docs there's mention of how such data should be encoded, the only guide is the JSONRPC documentation, which does ask for an 0x prefix for all hex data, but the user cannot be expected to be well informed about the lower level protocol to use the higher level JS API. Since the JS API already performs type conversion whenever convenient, it should fall into its responsibilities to handle this detail as well. It would also be helpful for web3.js to give a meaningful warning in case the prefix is omitted (as it's more sensible to include it anyway).

[epheph]

👍 to this. The current behavior in web3-1.0.0 is problematic. When deploying a contract at least, it simply chops off the first 2 bytes, likely assuming that it was just a "0x" without checking first, damaging the transaction and causing deploy issues (likely stack underflow)

[nivida]

When deploying a contract at least, it simply chops off the first 2 bytes, likely assuming that it was just a "0x" without checking first, damaging the transaction and causing deploy issues

I've checked that closer with version 1.2.4 of web3.js and I couldn't reproduce this issue. The web3.js library keeps the passed string as it is and doesn't remove any 0x prefix if passed with. Feel free to ping me here if I missed a case. @epheph @asmello.

[nivida]

@asmello I apologize for the confusion. The described behaviour in the comment of @epheph is not reproducible but there is still no validation of the inputs in 1.0 to ensure the prefix '0x' is existing. I will re-open this and immediately fix it.

[asmello]

Thanks! Glad this is finally being looked into. I haven't touched blockchain code in a while, but I'll stick around in case I can be of any help.

[nivida]

@asmello Thanks! But we sadly had to add the hold label to the related PR because it would break too many already existing DApps. We will merge this fix later for version 2.0 of web3.js.