Fix postsign (#4347)

* Fix whitespace.

* Fix post sign.

* Fix message.

* Fix tests.

* Rest of the problems.

* All hail the linter and its omniscience.

* ...and its divine omniscience.

* Grumbles and wording.
This commit is contained in:
Gav Wood 2017-01-30 15:08:02 +01:00 committed by GitHub
parent 5a18ed7c3e
commit ca196d683e
14 changed files with 94 additions and 53 deletions

View File

@ -297,6 +297,11 @@ export default class Parity {
.execute('parity_postTransaction', inOptions(options)); .execute('parity_postTransaction', inOptions(options));
} }
postSign (from, message) {
return this._transport
.execute('parity_postSign', from, message);
}
registryAddress () { registryAddress () {
return this._transport return this._transport
.execute('parity_registryAddress') .execute('parity_registryAddress')

View File

@ -61,7 +61,7 @@ export default class RequestPending extends Component {
address={ sign.address } address={ sign.address }
className={ className } className={ className }
focus={ focus } focus={ focus }
hash={ sign.hash } data={ sign.data }
id={ id } id={ id }
isFinished={ false } isFinished={ false }
isSending={ isSending } isSending={ isSending }

View File

@ -27,8 +27,20 @@
min-height: $pendingHeight; min-height: $pendingHeight;
} }
.signData {
border: 0.25em solid red;
padding: 0.5em;
margin-left: -2em;
overflow: auto;
max-height: 6em;
}
.signData > p {
color: white;
}
.signDetails { .signDetails {
flex: 1; flex: 10;
} }
.address, .info { .address, .info {

View File

@ -19,16 +19,30 @@ import { observer } from 'mobx-react';
import Account from '../Account'; import Account from '../Account';
import TransactionPendingForm from '../TransactionPendingForm'; import TransactionPendingForm from '../TransactionPendingForm';
import TxHashLink from '../TxHashLink';
import styles from './signRequest.css'; import styles from './signRequest.css';
function isAscii (data) {
for (var i = 2; i < data.length; i += 2) {
let n = parseInt(data.substr(i, 2), 16);
if (n < 32 || n >= 128) {
return false;
}
}
return true;
}
@observer @observer
export default class SignRequest extends Component { export default class SignRequest extends Component {
static contextTypes = {
api: PropTypes.object
};
static propTypes = { static propTypes = {
id: PropTypes.object.isRequired, id: PropTypes.object.isRequired,
address: PropTypes.string.isRequired, address: PropTypes.string.isRequired,
hash: PropTypes.string.isRequired, data: PropTypes.string.isRequired,
isFinished: PropTypes.bool.isRequired, isFinished: PropTypes.bool.isRequired,
isTest: PropTypes.bool.isRequired, isTest: PropTypes.bool.isRequired,
store: PropTypes.object.isRequired, store: PropTypes.object.isRequired,
@ -62,8 +76,23 @@ export default class SignRequest extends Component {
); );
} }
renderAsciiDetails (ascii) {
return (
<div className={ styles.signData }>
<p>{ascii}</p>
</div>
);
}
renderBinaryDetails (data) {
return (<div className={ styles.signData }>
<p>(Unknown binary data)</p>
</div>);
}
renderDetails () { renderDetails () {
const { address, hash, isTest, store } = this.props; const { api } = this.context;
const { address, isTest, store, data } = this.props;
const balance = store.balances[address]; const balance = store.balances[address];
if (!balance) { if (!balance) {
@ -79,9 +108,14 @@ export default class SignRequest extends Component {
isTest={ isTest } isTest={ isTest }
/> />
</div> </div>
<div className={ styles.info } title={ hash }> <div className={ styles.info } title={ api.util.sha3(data) }>
<p>Dapp is requesting to sign arbitrary transaction using this account.</p> <p>A request to sign data using your account:</p>
<p><strong>Confirm the transaction only if you trust the app.</strong></p> {
isAscii(data)
? this.renderAsciiDetails(api.util.hexToAscii(data))
: this.renderBinaryDetails(data)
}
<p><strong>WARNING: This consequences of doing this may be grave. Confirm the request only if you are sure.</strong></p>
</div> </div>
</div> </div>
); );
@ -92,19 +126,9 @@ export default class SignRequest extends Component {
if (isFinished) { if (isFinished) {
if (status === 'confirmed') { if (status === 'confirmed') {
const { hash, isTest } = this.props;
return ( return (
<div className={ styles.actions }> <div className={ styles.actions }>
<span className={ styles.isConfirmed }>Confirmed</span> <span className={ styles.isConfirmed }>Confirmed</span>
<div>
Transaction hash:
<TxHashLink
isTest={ isTest }
txHash={ hash }
className={ styles.txHash }
/>
</div>
</div> </div>
); );
} }

View File

@ -150,7 +150,7 @@ class TransactionPendingFormConfirm extends Component {
label={ label={
isSending isSending
? 'Confirming...' ? 'Confirming...'
: 'Confirm Transaction' : 'Confirm Request'
} }
onTouchTap={ this.onConfirm } onTouchTap={ this.onConfirm }
primary primary
@ -256,7 +256,8 @@ function mapStateToProps (_, initProps) {
return (state) => { return (state) => {
const { accounts } = state.personal; const { accounts } = state.personal;
const account = accounts[address] || {}; let gotAddress = Object.keys(accounts).find(a => a.toLowerCase() === address);
const account = gotAddress ? accounts[gotAddress] : {};
return { account }; return { account };
}; };

View File

@ -32,14 +32,14 @@ export default class TransactionPendingFormReject extends Component {
return ( return (
<div> <div>
<div className={ styles.rejectText }> <div className={ styles.rejectText }>
Are you sure you want to reject transaction? <br /> Are you sure you want to reject request? <br />
<strong>This cannot be undone</strong> <strong>This cannot be undone</strong>
</div> </div>
<RaisedButton <RaisedButton
onTouchTap={ onReject } onTouchTap={ onReject }
className={ styles.rejectButton } className={ styles.rejectButton }
fullWidth fullWidth
label={ 'Reject Transaction' } label={ 'Reject Request' }
/> />
</div> </div>
); );

View File

@ -75,7 +75,7 @@ export default class TransactionPendingForm extends Component {
let html; let html;
if (!isRejectOpen) { if (!isRejectOpen) {
html = <span>reject transaction</span>; html = <span>reject request</span>;
} else { } else {
html = <span><BackIcon />{ "I've changed my mind" }</span>; html = <span><BackIcon />{ "I've changed my mind" }</span>;
} }

View File

@ -19,6 +19,7 @@ use std::ops::Deref;
use rlp; use rlp;
use util::{Address, H256, U256, Uint, Bytes}; use util::{Address, H256, U256, Uint, Bytes};
use util::bytes::ToPretty; use util::bytes::ToPretty;
use util::sha3::Hashable;
use ethkey::Signature; use ethkey::Signature;
use ethcore::miner::MinerService; use ethcore::miner::MinerService;
@ -108,8 +109,8 @@ pub fn execute<C, M>(client: &C, miner: &M, accounts: &AccountProvider, payload:
.map(ConfirmationResponse::SignTransaction) .map(ConfirmationResponse::SignTransaction)
) )
}, },
ConfirmationPayload::Signature(address, hash) => { ConfirmationPayload::Signature(address, data) => {
signature(accounts, address, hash, pass) signature(accounts, address, data.sha3(), pass)
.map(|result| result .map(|result| result
.map(RpcH520::from) .map(RpcH520::from)
.map(ConfirmationResponse::Signature) .map(ConfirmationResponse::Signature)
@ -243,8 +244,8 @@ pub fn from_rpc<C, M>(payload: RpcConfirmationPayload, client: &C, miner: &M) ->
RpcConfirmationPayload::Decrypt(RpcDecryptRequest { address, msg }) => { RpcConfirmationPayload::Decrypt(RpcDecryptRequest { address, msg }) => {
ConfirmationPayload::Decrypt(address.into(), msg.into()) ConfirmationPayload::Decrypt(address.into(), msg.into())
}, },
RpcConfirmationPayload::Signature(RpcSignRequest { address, hash }) => { RpcConfirmationPayload::Signature(RpcSignRequest { address, data }) => {
ConfirmationPayload::Signature(address.into(), hash.into()) ConfirmationPayload::Signature(address.into(), data.into())
}, },
} }
} }

View File

@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License // You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>. // along with Parity. If not, see <http://www.gnu.org/licenses/>.
use util::{Address, U256, Bytes, H256}; use util::{Address, U256, Bytes};
/// Transaction request coming from RPC /// Transaction request coming from RPC
#[derive(Debug, Clone, Default, Eq, PartialEq, Hash)] #[derive(Debug, Clone, Default, Eq, PartialEq, Hash)]
@ -109,7 +109,7 @@ pub enum ConfirmationPayload {
/// Sign Transaction /// Sign Transaction
SignTransaction(FilledTransactionRequest), SignTransaction(FilledTransactionRequest),
/// Sign request /// Sign request
Signature(Address, H256), Signature(Address, Bytes),
/// Decrypt request /// Decrypt request
Decrypt(Address, Bytes), Decrypt(Address, Bytes),
} }

View File

@ -18,7 +18,7 @@
use std::sync::{Arc, Weak}; use std::sync::{Arc, Weak};
use transient_hashmap::TransientHashMap; use transient_hashmap::TransientHashMap;
use util::{U256, Mutex, Hashable}; use util::{U256, Mutex};
use ethcore::account_provider::AccountProvider; use ethcore::account_provider::AccountProvider;
use ethcore::miner::MinerService; use ethcore::miner::MinerService;
@ -122,9 +122,9 @@ impl<C: 'static, M: 'static> ParitySigning for SigningQueueClient<C, M> where
C: MiningBlockChainClient, C: MiningBlockChainClient,
M: MinerService, M: MinerService,
{ {
fn post_sign(&self, address: RpcH160, hash: RpcH256) -> Result<RpcEither<RpcU256, RpcConfirmationResponse>, Error> { fn post_sign(&self, address: RpcH160, data: RpcBytes) -> Result<RpcEither<RpcU256, RpcConfirmationResponse>, Error> {
self.active()?; self.active()?;
self.dispatch(RpcConfirmationPayload::Signature((address, hash).into())) self.dispatch(RpcConfirmationPayload::Signature((address, data).into()))
.map(|result| match result { .map(|result| match result {
DispatchResult::Value(v) => RpcEither::Or(v), DispatchResult::Value(v) => RpcEither::Or(v),
DispatchResult::Promise(promise) => { DispatchResult::Promise(promise) => {
@ -187,8 +187,7 @@ impl<C: 'static, M: 'static> EthSigning for SigningQueueClient<C, M> where
M: MinerService, M: MinerService,
{ {
fn sign(&self, address: RpcH160, data: RpcBytes) -> BoxFuture<RpcH520, Error> { fn sign(&self, address: RpcH160, data: RpcBytes) -> BoxFuture<RpcH520, Error> {
let hash = data.0.sha3().into(); let res = self.active().and_then(|_| self.dispatch(RpcConfirmationPayload::Signature((address, data).into())));
let res = self.active().and_then(|_| self.dispatch(RpcConfirmationPayload::Signature((address, hash).into())));
let (ready, p) = futures::oneshot(); let (ready, p) = futures::oneshot();
self.handle_dispatch(res, |response| { self.handle_dispatch(res, |response| {

View File

@ -17,7 +17,6 @@
//! Unsafe Signing RPC implementation. //! Unsafe Signing RPC implementation.
use std::sync::{Arc, Weak}; use std::sync::{Arc, Weak};
use util::Hashable;
use ethcore::account_provider::AccountProvider; use ethcore::account_provider::AccountProvider;
use ethcore::miner::MinerService; use ethcore::miner::MinerService;
@ -86,8 +85,7 @@ impl<C: 'static, M: 'static> EthSigning for SigningUnsafeClient<C, M> where
M: MinerService, M: MinerService,
{ {
fn sign(&self, address: RpcH160, data: RpcBytes) -> BoxFuture<RpcH520, Error> { fn sign(&self, address: RpcH160, data: RpcBytes) -> BoxFuture<RpcH520, Error> {
let hash = data.0.sha3().into(); let result = match self.handle(RpcConfirmationPayload::Signature((address, data).into())) {
let result = match self.handle(RpcConfirmationPayload::Signature((address, hash).into())) {
Ok(RpcConfirmationResponse::Signature(signature)) => Ok(signature), Ok(RpcConfirmationResponse::Signature(signature)) => Ok(signature),
Err(e) => Err(e), Err(e) => Err(e),
e => Err(errors::internal("Unexpected result", e)), e => Err(errors::internal("Unexpected result", e)),
@ -131,7 +129,7 @@ impl<C: 'static, M: 'static> ParitySigning for SigningUnsafeClient<C, M> where
futures::done(result).boxed() futures::done(result).boxed()
} }
fn post_sign(&self, _: RpcH160, _: RpcH256) -> Result<RpcEither<RpcU256, RpcConfirmationResponse>, Error> { fn post_sign(&self, _: RpcH160, _: RpcBytes) -> Result<RpcEither<RpcU256, RpcConfirmationResponse>, Error> {
// We don't support this in non-signer mode. // We don't support this in non-signer mode.
Err(errors::signer_disabled()) Err(errors::signer_disabled())
} }

View File

@ -85,14 +85,14 @@ fn should_return_list_of_items_to_confirm() {
nonce: None, nonce: None,
min_block: None, min_block: None,
})).unwrap(); })).unwrap();
tester.signer.add_request(ConfirmationPayload::Signature(1.into(), 5.into())).unwrap(); tester.signer.add_request(ConfirmationPayload::Signature(1.into(), vec![5].into())).unwrap();
// when // when
let request = r#"{"jsonrpc":"2.0","method":"signer_requestsToConfirm","params":[],"id":1}"#; let request = r#"{"jsonrpc":"2.0","method":"signer_requestsToConfirm","params":[],"id":1}"#;
let response = concat!( let response = concat!(
r#"{"jsonrpc":"2.0","result":["#, r#"{"jsonrpc":"2.0","result":["#,
r#"{"id":"0x1","payload":{"sendTransaction":{"data":"0x","from":"0x0000000000000000000000000000000000000001","gas":"0x989680","gasPrice":"0x2710","minBlock":null,"nonce":null,"to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","value":"0x1"}}},"#, r#"{"id":"0x1","payload":{"sendTransaction":{"data":"0x","from":"0x0000000000000000000000000000000000000001","gas":"0x989680","gasPrice":"0x2710","minBlock":null,"nonce":null,"to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","value":"0x1"}}},"#,
r#"{"id":"0x2","payload":{"sign":{"address":"0x0000000000000000000000000000000000000001","hash":"0x0000000000000000000000000000000000000000000000000000000000000005"}}}"#, r#"{"id":"0x2","payload":{"sign":{"address":"0x0000000000000000000000000000000000000001","data":"0x05"}}}"#,
r#"],"id":1}"# r#"],"id":1}"#
); );
@ -156,7 +156,7 @@ fn should_not_remove_transaction_if_password_is_invalid() {
fn should_not_remove_sign_if_password_is_invalid() { fn should_not_remove_sign_if_password_is_invalid() {
// given // given
let tester = signer_tester(); let tester = signer_tester();
tester.signer.add_request(ConfirmationPayload::Signature(0.into(), 5.into())).unwrap(); tester.signer.add_request(ConfirmationPayload::Signature(0.into(), vec![5].into())).unwrap();
assert_eq!(tester.signer.requests().len(), 1); assert_eq!(tester.signer.requests().len(), 1);
// when // when

View File

@ -18,7 +18,7 @@
use jsonrpc_core::Error; use jsonrpc_core::Error;
use futures::BoxFuture; use futures::BoxFuture;
use v1::types::{U256, H160, H256, Bytes, ConfirmationResponse, TransactionRequest, Either}; use v1::types::{U256, H160, Bytes, ConfirmationResponse, TransactionRequest, Either};
build_rpc_trait! { build_rpc_trait! {
/// Signing methods implementation. /// Signing methods implementation.
@ -26,7 +26,7 @@ build_rpc_trait! {
/// Posts sign request asynchronously. /// Posts sign request asynchronously.
/// Will return a confirmation ID for later use with check_transaction. /// Will return a confirmation ID for later use with check_transaction.
#[rpc(name = "parity_postSign")] #[rpc(name = "parity_postSign")]
fn post_sign(&self, H160, H256) -> Result<Either<U256, ConfirmationResponse>, Error>; fn post_sign(&self, H160, Bytes) -> Result<Either<U256, ConfirmationResponse>, Error>;
/// Posts transaction asynchronously. /// Posts transaction asynchronously.
/// Will return a transaction ID for later use with check_transaction. /// Will return a transaction ID for later use with check_transaction.

View File

@ -19,6 +19,7 @@
use std::fmt; use std::fmt;
use serde::{Serialize, Serializer}; use serde::{Serialize, Serializer};
use util::log::Colour; use util::log::Colour;
use util::bytes::ToPretty;
use v1::types::{U256, TransactionRequest, RichRawTransaction, H160, H256, H520, Bytes, BlockNumber}; use v1::types::{U256, TransactionRequest, RichRawTransaction, H160, H256, H520, Bytes, BlockNumber};
use v1::helpers; use v1::helpers;
@ -64,14 +65,14 @@ pub struct SignRequest {
/// Address /// Address
pub address: H160, pub address: H160,
/// Hash to sign /// Hash to sign
pub hash: H256, pub data: Bytes,
} }
impl From<(H160, H256)> for SignRequest { impl From<(H160, Bytes)> for SignRequest {
fn from(tuple: (H160, H256)) -> Self { fn from(tuple: (H160, Bytes)) -> Self {
SignRequest { SignRequest {
address: tuple.0, address: tuple.0,
hash: tuple.1, data: tuple.1,
} }
} }
} }
@ -80,8 +81,8 @@ impl fmt::Display for SignRequest {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!( write!(
f, f,
"sign 0x{:?} with {}", "sign 0x{} with {}",
self.hash, self.data.0.pretty(),
Colour::White.bold().paint(format!("0x{:?}", self.address)), Colour::White.bold().paint(format!("0x{:?}", self.address)),
) )
} }
@ -172,9 +173,9 @@ impl From<helpers::ConfirmationPayload> for ConfirmationPayload {
match c { match c {
helpers::ConfirmationPayload::SendTransaction(t) => ConfirmationPayload::SendTransaction(t.into()), helpers::ConfirmationPayload::SendTransaction(t) => ConfirmationPayload::SendTransaction(t.into()),
helpers::ConfirmationPayload::SignTransaction(t) => ConfirmationPayload::SignTransaction(t.into()), helpers::ConfirmationPayload::SignTransaction(t) => ConfirmationPayload::SignTransaction(t.into()),
helpers::ConfirmationPayload::Signature(address, hash) => ConfirmationPayload::Signature(SignRequest { helpers::ConfirmationPayload::Signature(address, data) => ConfirmationPayload::Signature(SignRequest {
address: address.into(), address: address.into(),
hash: hash.into(), data: data.into(),
}), }),
helpers::ConfirmationPayload::Decrypt(address, msg) => ConfirmationPayload::Decrypt(DecryptRequest { helpers::ConfirmationPayload::Decrypt(address, msg) => ConfirmationPayload::Decrypt(DecryptRequest {
address: address.into(), address: address.into(),
@ -248,12 +249,12 @@ mod tests {
// given // given
let request = helpers::ConfirmationRequest { let request = helpers::ConfirmationRequest {
id: 15.into(), id: 15.into(),
payload: helpers::ConfirmationPayload::Signature(1.into(), 5.into()), payload: helpers::ConfirmationPayload::Signature(1.into(), vec![5].into()),
}; };
// when // when
let res = serde_json::to_string(&ConfirmationRequest::from(request)); let res = serde_json::to_string(&ConfirmationRequest::from(request));
let expected = r#"{"id":"0xf","payload":{"sign":{"address":"0x0000000000000000000000000000000000000001","hash":"0x0000000000000000000000000000000000000000000000000000000000000005"}}}"#; let expected = r#"{"id":"0xf","payload":{"sign":{"address":"0x0000000000000000000000000000000000000001","data":"0x05"}}}"#;
// then // then
assert_eq!(res.unwrap(), expected.to_owned()); assert_eq!(res.unwrap(), expected.to_owned());