Panic on assigning to an account an extraordinary big balance in the chain spec

[aawaken]

• Parity Ethereum version: Parity-Ethereum/v2.5.10-stable-9f94473-20191112/x86_64-linux-gnu/rustc1.39.0
• Operating system: Linux
• Installation: one-line installer
• Fully synchronized: -
• Network: private PoA
• Restarted: yes

actual

Loading config file from config/local/node_init.toml

====================
stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: <unknown>
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: <unknown>
  15: <unknown>
  16: <unknown>
  17: __libc_start_main
  18: <unknown>

Thread 'main' panicked at 'assertion failed: 4 * 8 >= slice.len()', /ci-cache/parity-ethereum/cargo/build-linux/registry/src/github.com-1ecc6299db9ec823/ethereum-types-0.4.2/src/uint.rs:32

expected

1. Start normally or
2. return a helpful error message, something like "Invalid balance for account xy. Maximal allowed value: 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff (64 signs, 256 bit)"

steps to reproduce

start parity ethereum client with spec.json having the following starting account balance:
"0x00a1628c476b12ab9fc0ab95893bbc0170241521": { "balance": "0x10000000000000000000000000000000000000000000000000000000000000000"}

[lewisbelcher]

Seems that this affects all values being deserialised with uint::Uint. The following tests added to json/src/spec/account.rs both panic with the reported error:

#[test]
fn account_nonce_greater_than_u256_limit() {
	let s = r#"{
		"nonce": "0x10000000000000000000000000000000000000000000000000000000000000000"
	}"#;
	let _: Account = serde_json::from_str(s).unwrap();
}

#[test]
fn account_balance_greater_than_u256_limit() {
	let s = r#"{
		"balance": "0x10000000000000000000000000000000000000000000000000000000000000000"
	}"#;
	let _: Account = serde_json::from_str(s).unwrap();
}

In order to catch which were causing issues (especially down to the individual accounts granularity) we'd need to deserialise the components individually, right? Looks like they're all being deserialised together at the moment (https://github.com/paritytech/parity-ethereum/blob/master/ethcore/spec/src/spec.rs#L501). Also, Deserialize is being derived atm. Would we need to impl it instead?

P.s. apologies if I've missed something, I'm new to the codebase but wanted to start getting involved a little.

[dvdplm]

Start normally or

This doesn't seem possible: what value would you use? Cap to max? Cast to some new even wider int type?

If you agree with that, then I think the panic is warranted here: the program cannot proceed with such large values.

I agree that nice and helpful error messages are always nice, but in this case I wonder if it's really worth the effort? Maybe it's enough to check that we have documented the min/max of the Uint type properly.

[aawaken]

@dvdplm I agree about 1) - somewhere should be the cap for the max possible value. But still, if you have a broken config, it would be so helpful to see, what exactly is broken. Actually, the information given on the panic doesn't give any clue besides of that some uint value exceeds 32 bytes. I would find something like mentioned in 2) very helpful.

[niklasad1]

@aawaken

I just want to clarify that the root issue is not in parity ethereum because Uint is just a wrapper of ethereum_types::U256 which performs the heavy de-serialization. It has an ExpectSizeVisitor which you can find here that need to fixed properly to return the error accordingly.

However, for this case it is probably sufficient just to change https://github.com/paritytech/parity-ethereum/blob/master/json/src/uint.rs#L79 to be 3..=66

[aawaken]

Sorry, I leave the ticket management for you, guys :-)