Merge pull request #3633 from ethcore/fix-tx-rpc

Fix up the transaction JSON serialisation for RPC.

ethcore/src/types/transaction.rs

@@ -304,6 +304,9 @@ impl SignedTransaction {
 	/// 0 if `v` would have been 27 under "Electrum" notation, 1 if 28 or 4 if invalid.
 	pub fn standard_v(&self) -> u8 { match self.v { v if v == 27 || v == 28 || v > 36 => ((v - 1) % 2) as u8, _ => 4 } }
 
+	/// The `v` value that appears in the RLP.
+	pub fn original_v(&self) -> u64 { self.v }
+
 	/// The network ID, or `None` if this is a global transaction.
 	pub fn network_id(&self) -> Option<u64> {
 		match self.v {

rpc/src/v1/tests/mocked/eth.rs

@@ -494,7 +494,7 @@ fn rpc_eth_pending_transaction_by_hash() {
 		tester.miner.pending_transactions.lock().insert(H256::zero(), tx);
 	}
 
-	let response = r#"{"jsonrpc":"2.0","result":{"blockHash":null,"blockNumber":null,"creates":null,"from":"0x0f65fe9276bc9a24ae7083ae28e2660ef72df99e","gas":"0x5208","gasPrice":"0x1","hash":"0x41df922fd0d4766fcc02e161f8295ec28522f329ae487f14d811e4b64c8d6e31","input":"0x","nonce":"0x0","publicKey":"0x7ae46da747962c2ee46825839c1ef9298e3bd2e70ca2938495c3693a485ec3eaa8f196327881090ff64cf4fbb0a48485d4f83098e189ed3b7a87d5941b59f789","r":"0x48b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353","raw":"0xf85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","s":"0xefffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","to":"0x095e7baea6a6c7c4c2dfeb977efac326af552d87","transactionIndex":null,"v":0,"value":"0xa"},"id":1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":{"blockHash":null,"blockNumber":null,"creates":null,"from":"0x0f65fe9276bc9a24ae7083ae28e2660ef72df99e","gas":"0x5208","gasPrice":"0x1","hash":"0x41df922fd0d4766fcc02e161f8295ec28522f329ae487f14d811e4b64c8d6e31","input":"0x","networkId":null,"nonce":"0x0","publicKey":"0x7ae46da747962c2ee46825839c1ef9298e3bd2e70ca2938495c3693a485ec3eaa8f196327881090ff64cf4fbb0a48485d4f83098e189ed3b7a87d5941b59f789","r":"0x48b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353","raw":"0xf85f800182520894095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba048b55bfa915ac795c431978d8a6a992b628d557da5ff759b307d495a36649353a0efffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","s":"0xefffd310ac743f371de3b9f7f9cb56c0b28ad43601b4ab949f53faa07bd2c804","standardV":"0x0","to":"0x095e7baea6a6c7c4c2dfeb977efac326af552d87","transactionIndex":null,"v":"0x1b","value":"0xa"},"id":1}"#;
 	let request = r#"{
 		"jsonrpc": "2.0",
 		"method": "eth_getTransactionByHash",
@@ -809,13 +809,16 @@ fn rpc_eth_sign_transaction() {
 		&format!("\"from\":\"0x{:?}\",", &address) +
 		r#""gas":"0x76c0","gasPrice":"0x9184e72a000","# +
 		&format!("\"hash\":\"0x{:?}\",", t.hash()) +
-		r#""input":"0x","nonce":"0x1","# +
+		r#""input":"0x","# +
+		&format!("\"networkId\":{},", t.network_id().map_or("null".to_owned(), |n| format!("{}", n))) +
+		r#""nonce":"0x1","# +
 		&format!("\"publicKey\":\"0x{:?}\",", t.public_key().unwrap()) +
-		&format!("\"r\":\"0x{}\",", signature.r().to_hex()) +
+		&format!("\"r\":\"0x{}\",", U256::from(signature.r()).to_hex()) +
 		&format!("\"raw\":\"0x{}\",", rlp.to_hex()) +
-		&format!("\"s\":\"0x{}\",", signature.s().to_hex()) +
+		&format!("\"s\":\"0x{}\",", U256::from(signature.s()).to_hex()) +
+		&format!("\"standardV\":\"0x{}\",", U256::from(t.standard_v()).to_hex()) +
 		r#""to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","transactionIndex":null,"# +
-		&format!("\"v\":{},", signature.v()) +
+		&format!("\"v\":\"0x{}\",", U256::from(t.original_v()).to_hex()) +
 		r#""value":"0x9184e72a""# +
 		r#"}},"id":1}"#;
 

rpc/src/v1/tests/mocked/signing.rs

@@ -284,13 +284,16 @@ fn should_add_sign_transaction_to_the_queue() {
 		&format!("\"from\":\"0x{:?}\",", &address) +
 		r#""gas":"0x76c0","gasPrice":"0x9184e72a000","# +
 		&format!("\"hash\":\"0x{:?}\",", t.hash()) +
-		r#""input":"0x","nonce":"0x1","# +
+		r#""input":"0x","# +
+		&format!("\"networkId\":{},", t.network_id().map_or("null".to_owned(), |n| format!("{}", n))) +
+		r#""nonce":"0x1","# +
 		&format!("\"publicKey\":\"0x{:?}\",", t.public_key().unwrap()) +
-		&format!("\"r\":\"0x{}\",", signature.r().to_hex()) +
+		&format!("\"r\":\"0x{}\",", U256::from(signature.r()).to_hex()) +
 		&format!("\"raw\":\"0x{}\",", rlp.to_hex()) +
-		&format!("\"s\":\"0x{}\",", signature.s().to_hex()) +
+		&format!("\"s\":\"0x{}\",", U256::from(signature.s()).to_hex()) +
+		&format!("\"standardV\":\"0x{}\",", U256::from(t.standard_v()).to_hex()) +
 		r#""to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","transactionIndex":null,"# +
-		&format!("\"v\":{},", signature.v()) +
+		&format!("\"v\":\"0x{}\",", U256::from(t.original_v()).to_hex()) +
 		r#""value":"0x9184e72a""# +
 		r#"}},"id":1}"#;
 

rpc/src/v1/types/block.rs

@@ -139,7 +139,7 @@ mod tests {
 	fn test_serialize_block_transactions() {
 		let t = BlockTransactions::Full(vec![Transaction::default()]);
 		let serialized = serde_json::to_string(&t).unwrap();
-		assert_eq!(serialized, r#"[{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0","blockHash":null,"blockNumber":null,"transactionIndex":null,"from":"0x0000000000000000000000000000000000000000","to":null,"value":"0x0","gasPrice":"0x0","gas":"0x0","input":"0x","creates":null,"raw":"0x","publicKey":null,"v":0,"r":"0x0000000000000000000000000000000000000000000000000000000000000000","s":"0x0000000000000000000000000000000000000000000000000000000000000000"}]"#);
+		assert_eq!(serialized, r#"[{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0","blockHash":null,"blockNumber":null,"transactionIndex":null,"from":"0x0000000000000000000000000000000000000000","to":null,"value":"0x0","gasPrice":"0x0","gas":"0x0","input":"0x","creates":null,"raw":"0x","publicKey":null,"networkId":null,"standardV":"0x0","v":"0x0","r":"0x0","s":"0x0"}]"#);
 
 		let t = BlockTransactions::Hashes(vec![H256::default().into()]);
 		let serialized = serde_json::to_string(&t).unwrap();

rpc/src/v1/types/transaction.rs

@@ -57,12 +57,18 @@ pub struct Transaction {
 	/// Public key of the signer.
 	#[serde(rename="publicKey")]
 	pub public_key: Option<H512>,
-	/// The V field of the signature.
+	/// The network id of the transaction, if any.
-	pub v: u8,
+	#[serde(rename="networkId")]
+	pub network_id: Option<u64>,
+	/// The standardised V field of the signature (0 or 1).
+	#[serde(rename="standardV")]
+	pub standard_v: U256,
+	/// The standardised V field of the signature.
+	pub v: U256,
 	/// The R field of the signature.
-	pub r: H256,
+	pub r: U256,
 	/// The S field of the signature.
-	pub s: H256,
+	pub s: U256,
 }
 
 /// Local Transaction Status
@@ -176,7 +182,9 @@ impl From<LocalizedTransaction> for Transaction {
 			},
 			raw: ::rlp::encode(&t.signed).to_vec().into(),
 			public_key: t.public_key().ok().map(Into::into),
-			v: signature.v(),
+			network_id: t.network_id(),
+			standard_v: t.standard_v().into(),
+			v: t.original_v().into(),
 			r: signature.r().into(),
 			s: signature.s().into(),
 		}
@@ -207,7 +215,9 @@ impl From<SignedTransaction> for Transaction {
 			},
 			raw: ::rlp::encode(&t).to_vec().into(),
 			public_key: t.public_key().ok().map(Into::into),
-			v: signature.v(),
+			network_id: t.network_id(),
+			standard_v: t.standard_v().into(),
+			v: t.original_v().into(),
 			r: signature.r().into(),
 			s: signature.s().into(),
 		}
@@ -238,7 +248,7 @@ mod tests {
 	fn test_transaction_serialize() {
 		let t = Transaction::default();
 		let serialized = serde_json::to_string(&t).unwrap();
-		assert_eq!(serialized, r#"{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0","blockHash":null,"blockNumber":null,"transactionIndex":null,"from":"0x0000000000000000000000000000000000000000","to":null,"value":"0x0","gasPrice":"0x0","gas":"0x0","input":"0x","creates":null,"raw":"0x","publicKey":null,"v":0,"r":"0x0000000000000000000000000000000000000000000000000000000000000000","s":"0x0000000000000000000000000000000000000000000000000000000000000000"}"#);
+		assert_eq!(serialized, r#"{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0","blockHash":null,"blockNumber":null,"transactionIndex":null,"from":"0x0000000000000000000000000000000000000000","to":null,"value":"0x0","gasPrice":"0x0","gas":"0x0","input":"0x","creates":null,"raw":"0x","publicKey":null,"networkId":null,"standardV":"0x0","v":"0x0","r":"0x0","s":"0x0"}"#);
 	}
 
 	#[test]

rpc/src/v1/types/uint.rs

@@ -14,9 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-use std::cmp;
 use std::str::FromStr;
-use rustc_serialize::hex::ToHex;
 use serde;
 use util::{U256 as EthU256, U128 as EthU128, Uint};
 
@@ -50,18 +48,7 @@ macro_rules! impl_uint {
 
 		impl serde::Serialize for $name {
 			fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> where S: serde::Serializer {
-				let mut hex = "0x".to_owned();
+				serializer.serialize_str(&format!("0x{}", self.0.to_hex()))
-				let mut bytes = [0u8; 8 * $size];
-				self.0.to_big_endian(&mut bytes);
-				let len = cmp::max((self.0.bits() + 7) / 8, 1);
-				let bytes_hex = bytes[bytes.len() - len..].to_hex();
-
-				if bytes_hex.starts_with('0') {
-					hex.push_str(&bytes_hex[1..]);
-				} else {
-					hex.push_str(&bytes_hex);
-				}
-				serializer.serialize_str(&hex)
 			}
 		}
 

util/bigint/src/uint.rs

@@ -37,12 +37,12 @@
 //! implementations for even more speed, hidden behind the `x64_arithmetic`
 //! feature flag.
 
-use std::{mem, fmt};
+use std::{mem, fmt, cmp};
 use std::str::{FromStr};
 use std::hash::Hash;
 use std::ops::{Shr, Shl, BitAnd, BitOr, BitXor, Not, Div, Rem, Mul, Add, Sub};
 use std::cmp::Ordering;
-use rustc_serialize::hex::{FromHex, FromHexError};
+use rustc_serialize::hex::{ToHex, FromHex, FromHexError};
 
 /// Conversion from decimal string error
 #[derive(Debug, PartialEq)]
@@ -520,8 +520,10 @@ pub trait Uint: Sized + Default + FromStr + From<u64> + fmt::Debug + fmt::Displa
 	fn bit(&self, index: usize) -> bool;
 	/// Return single byte
 	fn byte(&self, index: usize) -> u8;
-	/// Convert U256 to the sequence of bytes with a big endian
+	/// Convert to the sequence of bytes with a big endian
 	fn to_big_endian(&self, bytes: &mut[u8]);
+	/// Convert to a non-zero-prefixed hex representation (not prefixed by `0x`). 
+	fn to_hex(&self) -> String;
 	/// Create `Uint(10**n)`
 	fn exp10(n: usize) -> Self;
 	/// Return eponentation `self**other`. Panic on overflow.
@@ -684,6 +686,17 @@ macro_rules! construct_uint {
 				}
 			}
 
+			#[inline]
+			fn to_hex(&self) -> String {
+				if self.is_zero() { return "0".to_owned(); }	// special case.
+				let mut bytes = [0u8; 8 * $n_words];
+				self.to_big_endian(&mut bytes);
+				let bp7 = self.bits() + 7;
+				let len = cmp::max(bp7 / 8, 1);
+				let bytes_hex = bytes[bytes.len() - len..].to_hex();
+				(&bytes_hex[1 - bp7 % 8 / 4..]).to_owned()
+			}
+
 			#[inline]
 			fn exp10(n: usize) -> Self {
 				match n {
@@ -1651,6 +1664,18 @@ mod tests {
 		U256::from(2).pow(U256::from(0x100));
 	}
 
+	#[test]
+	fn should_format_hex_correctly() {
+		assert_eq!(&U256::from(0).to_hex(), &"0");
+		assert_eq!(&U256::from(0x1).to_hex(), &"1");
+		assert_eq!(&U256::from(0xf).to_hex(), &"f");
+		assert_eq!(&U256::from(0x10).to_hex(), &"10");
+		assert_eq!(&U256::from(0xff).to_hex(), &"ff");
+		assert_eq!(&U256::from(0x100).to_hex(), &"100");
+		assert_eq!(&U256::from(0xfff).to_hex(), &"fff");
+		assert_eq!(&U256::from(0x1000).to_hex(), &"1000");
+	}
+
 	#[test]
 	fn uint256_overflowing_pow() {
 		// assert_eq!(