Add block & timestamp conditions to Signer #4411

jacogr · 2017-02-02T17:09:58Z

Allows the editing of min transmission block or minimum transaction timestamp from Signer, Send Transaction & Execute Contract (anywhere where the GasPriceEditor component was available)
Depends on Parity support for condition: { block: <some-blockNumber> } & condition: { time: <ms-since-1970> } (https://github.com/ethcore/parity/pull/4419)

Still TODO (Future work) -

Rename GasPriceEditor to TxEditor (fits in closer to actual use now)
Adjust styling of the Date/Time popups (they don't quite fit)
Add tests for the UI components added
Add tests for RPC formatters
Expand tests for store transaction overrides

ngotchac

Comments made code-wise. Still need to fully test when #4419 is merged

ngotchac · 2017-02-03T16:07:43Z

js/src/api/format/input.js

+    if (condition.block) {
+      condition.block = condition.block ? inNumber10(condition.block) : null;
+    } else if (condition.time) {
+      condition.time = inNumber10(Math.floor(condition.time.getTime() / 1000));


Is there any other RPC calls with time/date involved ? Might be useful to have an inTime formatter to make it DRY

ngotchac · 2017-02-03T16:11:04Z

js/src/modals/Transfer/store.js

@@ -639,15 +624,12 @@ export default class TransferStore {
    const to = (isEth && !isWallet) ? this.recipient
      : (this.isWallet ? this.wallet.address : this.token.address);

-    const options = {
+    const options = this.gasStore.overrideTransaction({


applyToTransaction maybe ?

I didn't want to change the naming anywhere, so kept it as-is. If it bothers, can do it once there is no pressure.

ngotchac · 2017-02-03T16:11:21Z

js/src/api/format/output.js

@@ -221,6 +221,18 @@ export function outSyncing (syncing) {
  return syncing;
 }

+export function outTransactionCondition (condition) {


As above. No tests, not in the timeframe allowed.

ngotchac · 2017-02-03T16:11:27Z

js/src/api/format/input.js

@@ -127,6 +127,18 @@ export function inNumber16 (number) {
  return inHex(bn.toString(16));
 }

+export function inOptionsCondition (condition) {


Honestly don't have the space to add it here for release. Hence the comment in the PR - I need tests for all of these.

Fair enough ! 👍

ngotchac · 2017-02-03T16:15:55Z

js/src/ui/Form/InputDate/inputDate.js

+import styles from './inputDate.css';
+
+// NOTE: Has to be larger than Signer overlay Z, aligns with ../InputTime
+const DIALOG_STYLE = { zIndex: 10010 };


Not for this PR, but shouldn't we have some kind of central file to list all the zIndexes ? Would be easier to get a grasp of what's on top of what

ngotchac · 2017-02-03T16:16:23Z

js/src/ui/Form/InputDate/inputDate.js

+      <div className={ [styles.container, className].join(' ') }>
+        <Label label={ label } />
+        <DatePicker
+          autoOk


Best prop name ! 👍

ngotchac · 2017-02-03T16:23:59Z

js/src/ui/MethodDecoding/methodDecoding.js

-    );
+    if (condition.block && condition.block.gt(0)) {
+      return (
+        <span>, { historic ? 'Submitted' : 'Submission' } at block <span className={ styles.highlight }>#{ condition.block.toFormat(0) }</span></span>


Use FormattedMessage ?

Historic, didn't want to change - once I do I need to do the whole file, tried to keep changes to the bare minimum.

The more that gets changed, the more that can go wrong.

I actually have started with the MethodDecoding -> FormattedMessage - this file is arguably the worst I've seen out of all the ones I've done. It is not straight forward, will required a lot of (useful) refactoring to get right.

ngotchac · 2017-02-03T16:24:11Z

js/src/ui/MethodDecoding/methodDecoding.js

+
+    if (condition.time) {
+      return (
+        <span>, { historic ? 'Submitted' : 'Submission' } at <span className={ styles.highlight }>{ moment(condition.time).format('LLLL') }</span></span>


Use FormattedMessage ?

ngotchac · 2017-02-03T16:24:27Z

js/src/views/Signer/components/TransactionMainDetails/transactionMainDetails.js

@@ -91,7 +91,7 @@ export default class TransactionMainDetails extends Component {
      <div className={ styles.editButtonRow }>
        <Button
          icon={ <MapsLocalGasStation /> }
-          label='Edit gas/gasPrice'
+          label='Edit conditions/gas/gasPrice'


Use FormattedMessage ?

ngotchac · 2017-02-03T16:39:36Z

All of the comments have been addressed.
Some UI issues:

The block number input is not a Number/Integer input (can't use <up> and <down> the augment/decrement)
The block number input should have a default value of the current block number
There should be an option to reset this block number input to the current block number

jacogr · 2017-02-03T17:04:51Z

Ok, going to put those in the "I'm not fixing that here" bucket.

blockNumbers function exactly the way it has been in the previous iteration, only difference is the component they are rendered on.
blockNumber is not available directly in current components, so would have to bring in some more sweeping changes (did briefly look at it - I believe the min block should be fixed to the current blockNumber as well, instead of allowing all the way down to 0 as it currently does)
Changing the blockNumber input to TypedInput means we would need to do it for the rest of the elements (w.g. gas/gasPrice) as well. Nice enhancement, not going to take the risk here since I believe the gas/gasPrice itself has some assumptions about what they get in.
Like the reset option idea. (Not here, enhancement territory)

Help always appreciated, all these are great to have, not critical to get the feature out and usable.

ngotchac · 2017-02-03T17:31:10Z

@jacogr This should work :

We should at least include the Input type as number with a minimum set to 1.
If think it's safe to include the rest as well.. Your call

(NB : the store prop had to be renamed to gasStore because of React Redux using it's own store as a prop)

diff --git a/js/src/modals/ExecuteContract/AdvancedStep/advancedStep.js b/js/src/modals/ExecuteContract/AdvancedStep/advancedStep.js
index 268ca1f..b7d8918 100644
--- a/js/src/modals/ExecuteContract/AdvancedStep/advancedStep.js
+++ b/js/src/modals/ExecuteContract/AdvancedStep/advancedStep.js
@@ -30,7 +30,7 @@ export default class AdvancedStep extends Component {
 
     return (
       <div className={ styles.gaseditor }>
-        <GasPriceEditor store={ gasStore } />
+        <GasPriceEditor gasStore={ gasStore } />
       </div>
     );
   }
diff --git a/js/src/modals/Transfer/Extras/extras.js b/js/src/modals/Transfer/Extras/extras.js
index 61b99b2..253b324 100644
--- a/js/src/modals/Transfer/Extras/extras.js
+++ b/js/src/modals/Transfer/Extras/extras.js
@@ -40,7 +40,7 @@ export default class Extras extends Component {
         { this.renderData() }
         <div className={ styles.gaseditor }>
           <GasPriceEditor
-            store={ gasStore }
+            gasStore={ gasStore }
             onChange={ onChange }
           />
         </div>
diff --git a/js/src/ui/GasPriceEditor/gasPriceEditor.js b/js/src/ui/GasPriceEditor/gasPriceEditor.js
index 35723bd..81ca80f 100644
--- a/js/src/ui/GasPriceEditor/gasPriceEditor.js
+++ b/js/src/ui/GasPriceEditor/gasPriceEditor.js
@@ -18,6 +18,7 @@ import BigNumber from 'bignumber.js';
 import { observer } from 'mobx-react';
 import React, { Component, PropTypes } from 'react';
 import { FormattedMessage } from 'react-intl';
+import { connect } from 'react-redux';
 
 import { Input, InputDate, InputTime, RadioButtons } from '../Form';
 import GasPriceSelector from '../GasPriceSelector';
@@ -56,23 +57,38 @@ const CONDITION_VALUES = [
 ];
 
 @observer
-export default class GasPriceEditor extends Component {
+class GasPriceEditor extends Component {
   static contextTypes = {
     api: PropTypes.object.isRequired
   };
 
   static propTypes = {
+    blockNumber: PropTypes.object.isRequired,
     children: PropTypes.node,
-    onChange: PropTypes.func,
-    store: PropTypes.object.isRequired
+    gasStore: PropTypes.object.isRequired,
+    onChange: PropTypes.func
   }
 
   static Store = Store;
 
+  componentWillMount () {
+    const { blockNumber, gasStore } = this.props;
+
+    gasStore.updateBlockNumber(blockNumber);
+  }
+
+  componentWillReceiveProps (nextProps) {
+    if (!this.props.blockNumber.equals(nextProps.blockNumber)) {
+      const { blockNumber, gasStore } = nextProps;
+
+      gasStore.updateBlockNumber(blockNumber);
+    }
+  }
+
   render () {
     const { api } = this.context;
-    const { children, store } = this.props;
-    const { conditionType, errorGas, errorPrice, errorTotal, estimated, gas, histogram, price, priceDefault, totalValue } = store;
+    const { children, gasStore } = this.props;
+    const { conditionType, errorGas, errorPrice, errorTotal, estimated, gas, histogram, price, priceDefault, totalValue } = gasStore;
 
     const eth = api.util.fromWei(totalValue).toFormat();
     const gasLabel = `gas (estimated: ${new BigNumber(estimated).toFormat()})`;
@@ -145,7 +161,8 @@ export default class GasPriceEditor extends Component {
   }
 
   renderConditions () {
-    const { conditionType, condition, conditionBlockError } = this.props.store;
+    const { blockNumber = 0 } = this.props;
+    const { conditionType, condition, conditionBlockError } = this.props.gasStore;
 
     if (conditionType === CONDITIONS.NONE) {
       return null;
@@ -170,6 +187,9 @@ export default class GasPriceEditor extends Component {
                 />
               }
               onChange={ this.onChangeConditionBlock }
+              min={ blockNumber }
+              step={ 1 }
+              type='number'
               value={ condition.block }
             />
           </div>
@@ -220,28 +240,43 @@ export default class GasPriceEditor extends Component {
   }
 
   onEditGas = (event, gas) => {
-    const { store, onChange } = this.props;
+    const { gasStore, onChange } = this.props;
 
-    store.setGas(gas);
+    gasStore.setGas(gas);
     onChange && onChange('gas', gas);
   }
 
   onEditGasPrice = (event, price) => {
-    const { store, onChange } = this.props;
+    const { gasStore, onChange } = this.props;
 
-    store.setPrice(price);
+    gasStore.setPrice(price);
     onChange && onChange('gasPrice', price);
   }
 
   onChangeConditionType = (conditionType) => {
-    this.props.store.setConditionType(conditionType.key);
+    this.props.gasStore.setConditionType(conditionType.key);
   }
 
   onChangeConditionBlock = (event, blockNumber) => {
-    this.props.store.setConditionBlockNumber(blockNumber);
+    this.props.gasStore.setConditionBlockNumber(blockNumber);
   }
 
   onChangeConditionDateTime = (event, datetime) => {
-    this.props.store.setConditionDateTime(datetime);
+    this.props.gasStore.setConditionDateTime(datetime);
   }
 }
+
+// export default GasPriceEditor;
+
+function mapStateToProps (state) {
+  const { blockNumber } = state.nodeStatus;
+
+  return {
+    blockNumber
+  };
+}
+
+export default connect(
+  mapStateToProps,
+  null
+)(GasPriceEditor);
diff --git a/js/src/ui/GasPriceEditor/gasPriceEditor.spec.js b/js/src/ui/GasPriceEditor/gasPriceEditor.spec.js
index 9a42e89..a2337a2 100644
--- a/js/src/ui/GasPriceEditor/gasPriceEditor.spec.js
+++ b/js/src/ui/GasPriceEditor/gasPriceEditor.spec.js
@@ -40,7 +40,7 @@ const store = {
 describe('ui/GasPriceEditor', () => {
   it('renders', () => {
     expect(shallow(
-      <GasPriceEditor store={ store } />,
+      <GasPriceEditor gasStore={ store } />,
       { context: { api } }
     )).to.be.ok;
   });
diff --git a/js/src/ui/GasPriceEditor/store.js b/js/src/ui/GasPriceEditor/store.js
index fb35f5d..53ce42e 100644
--- a/js/src/ui/GasPriceEditor/store.js
+++ b/js/src/ui/GasPriceEditor/store.js
@@ -80,7 +80,7 @@ export default class GasPriceEditor {
 
       switch (conditionType) {
         case CONDITIONS.BLOCK:
-          this.condition = Object.assign({}, this.condition, { block: 0 });
+          this.condition = Object.assign({}, this.condition, { block: this.blockNumber || 0 });
           break;
 
         case CONDITIONS.TIME:
@@ -201,6 +201,10 @@ export default class GasPriceEditor {
       });
   }
 
+  updateBlockNumber (blockNumber) {
+    this.blockNumber = blockNumber.toNumber();
+  }
+
   overrideTransaction = (transaction) => {
     if (this.errorGas || this.errorPrice || this.conditionBlockError) {
       return transaction;
diff --git a/js/src/views/Signer/components/TransactionPending/transactionPending.js b/js/src/views/Signer/components/TransactionPending/transactionPending.js
index a49f5c2..28d7356 100644
--- a/js/src/views/Signer/components/TransactionPending/transactionPending.js
+++ b/js/src/views/Signer/components/TransactionPending/transactionPending.js
@@ -122,7 +122,7 @@ export default class TransactionPending extends Component {
 
     return (
       <div className={ `${styles.container} ${className}` }>
-        <GasPriceEditor store={ this.gasStore }>
+        <GasPriceEditor gasStore={ this.gasStore }>
           <Button
             label='view transaction'
             onClick={ this.toggleGasEditor }

jacogr · 2017-02-03T17:35:32Z

I like it. I'm open to the type=number option as a stop-gap (better, non-intrusive - should be easily testable.) The rest I'm happy to merge in with master after this PR after some additional testing.

Will apply the number and see if anything breaks.

jacogr · 2017-02-03T17:46:17Z

Only edited on Transfer and blockNumber/gas/gasPrice seems ok.

ngotchac · 2017-02-03T18:00:33Z

Ok, what about this ?
Seems to work fine, only 3 lines added

diff --git a/js/src/ui/GasPriceEditor/store.js b/js/src/ui/GasPriceEditor/store.js
index 3d6931d..e7c0ae8 100644
--- a/js/src/ui/GasPriceEditor/store.js
+++ b/js/src/ui/GasPriceEditor/store.js
@@ -62,6 +62,9 @@ export default class GasPriceEditor {
 
     if (api) {
       this.loadDefaults();
+      api.eth.blockNumber().then((blockNumber) => {
+        this.blockNumber = blockNumber.toNumber();
+      });
     }
   }
 
@@ -80,7 +83,7 @@ export default class GasPriceEditor {
 
       switch (conditionType) {
         case CONDITIONS.BLOCK:
-          this.condition = Object.assign({}, this.condition, { block: 1 });
+          this.condition = Object.assign({}, this.condition, { block: this.blockNumber || 1 });
           break;
 
         case CONDITIONS.TIME:

ngotchac · 2017-02-03T18:13:35Z

(Just need to make sure it works ok with #4419)

jacogr · 2017-02-03T19:01:04Z

Merging as-is so testing with beta build can be done.

* s/Delete Contract/Forget Contract/ (#4237) * Adjust the location of the signer snippet (#4155) * Additional building-block UI components (#4239) * Currency WIP * Expand tests * Pass className * Add QrCode * Export new components in ~/ui * s/this.props.netSymbol/netSymbol/ * Fix import case * ui/SectionList component (#4292) * array chunking utility * add SectionList component * Add TODOs to indicate possible future work * Add missing overlay style (as used in dapps at present) * Add a Playground for the UI Components (#4301) * Playground // WIP * Linting * Add Examples with code * CSS Linting * Linting * Add Connected Currency Symbol * 2015-2017 * 2015-2017 * 2015-2017 * 2015-2017 * 2015-2017 * 2015-2017 * 2015-2017 * Added `renderSymbol` tests * PR grumbles * Add Eth and Btc QRCode examples * 2015-2017 * Add tests for playground * Fixing tests * Split Dapp icon into ui/DappIcon (#4308) * Add QrCode & Copy to ShapeShift (#4322) * Extract CopyIcon to ~/ui/Icons * Add copy & QrCode address * Default size 4 * Add bitcoin: link * use protocol links applicable to coin exchanged * Remove .only * Display QrCode for accounts, addresses & contracts (#4329) * Allow Portal to be used as top-level modal (#4338) * Portal * Allow Portal to be used in as both top-level and popover * modal/popover variable naming * export Portal in ~/ui * Properly handle optional onKeyDown * Add simple Playground Example * Add proper event listener to Portal (#4359) * Display AccountCard name via IdentityName (#4235) * Fix signing (#4363) * Dapp Account Selection & Defaults (#4355) * Add parity_defaultAccount RPC (with subscription) (#4383) * Default Account selector in Signer overlay (#4375) * Typo, fixes #4271 (#4391) * Fix ParityBar account selection overflows (#4405) * Available Dapp selection alignment with Permissions (Portal) (#4374) * registry dapp: make lookup use lower case (#4409) * Dapps use defaultAccount instead of own selectors (#4386) * Poll for defaultAccount to update dapp & overlay subscriptions (#4417) * Poll for defaultAccount (Fixes #4413) * Fix nextTimeout on catch * Store timers * Re-enable default updates on change detection * Add block & timestamp conditions to Signer (#4411) * Extension installation overlay (#4423) * Extension installation overlay * Pr gumbles * Spelling * Update Chrome URL * Fix for non-included jsonrpc * Extend Portal component (as per Modal) #4392

jacogr added 2 commits February 2, 2017 18:03

WIP

b2cbba7

WIP (with lint)

8fd5950

jacogr added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M7-ui labels Feb 2, 2017

jacogr added 8 commits February 2, 2017 21:12

Update ui/RadioButtons

31083a2

transaction.condition

b29483a

Merge branch 'master' into jg-signer-timestamp

913e2a4

Date & Time selection in-place

6cd5560

Merge branch 'master' into jg-signer-timestamp

41aa0ef

Swap to condition-only

45dff4a

Fix tests, align naming

0c22a90

Pick error properly from validation

70dd843

Update tests

a996ac8

jacogr added B0-patch and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 3, 2017

jacogr added 4 commits February 3, 2017 13:56

condition: time sent withough ms

b9b9d2a

Merge branch 'master' into jg-signer-timestamp

65a9c27

Format numbers as base-10

b01efc5

override popup styles (zIndex)

786f89c

jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Feb 3, 2017

jacogr added 3 commits February 3, 2017 14:40

Merge branch 'master' into jg-signer-timestamp

fe38d57

Pass condition to signer

b7eef53

Update expectation (failing test typo)

0ba4d04

ngotchac reviewed Feb 3, 2017

View reviewed changes

Adjust min/max height for expanded bar

7bf4d24

ngotchac added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 3, 2017

jacogr added 3 commits February 3, 2017 17:48

Fix address display

58244b3

Merge branch 'jg-fix-add' into jg-signer-timestamp

8ed61f8

Fix name display

497edbb

jacogr mentioned this pull request Feb 3, 2017

TxEditor number editing #4425

Closed

jacogr added 2 commits February 3, 2017 18:44

Number inputs for gas/gasPrice/blockNumber

1af0ec2

Default blockNumber to 1 (align with min setting)

a6ad949

Update tests with min value

74651bd

Add Block Number

d6ae415

ngotchac added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Feb 3, 2017

Fix failing tests (after blockNumber intro)

06dd31f

jacogr merged commit cd4d489 into master Feb 3, 2017

jacogr deleted the jg-signer-timestamp branch February 3, 2017 19:01

jacogr added a commit that referenced this pull request Feb 3, 2017

Add block & timestamp conditions to Signer (#4411)

532567d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add block & timestamp conditions to Signer #4411

Add block & timestamp conditions to Signer #4411

jacogr commented Feb 2, 2017 •

edited

Loading

ngotchac left a comment

ngotchac Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

ngotchac Feb 3, 2017

ngotchac Feb 3, 2017

ngotchac Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

jacogr Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

ngotchac commented Feb 3, 2017

jacogr commented Feb 3, 2017

ngotchac commented Feb 3, 2017 •

edited

Loading

jacogr commented Feb 3, 2017

jacogr commented Feb 3, 2017

ngotchac commented Feb 3, 2017 •

edited

Loading

ngotchac commented Feb 3, 2017 •

edited

Loading

jacogr commented Feb 3, 2017

Add block & timestamp conditions to Signer #4411

Add block & timestamp conditions to Signer #4411

Conversation

jacogr commented Feb 2, 2017 • edited Loading

ngotchac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngotchac commented Feb 3, 2017

jacogr commented Feb 3, 2017

ngotchac commented Feb 3, 2017 • edited Loading

jacogr commented Feb 3, 2017

jacogr commented Feb 3, 2017

ngotchac commented Feb 3, 2017 • edited Loading

ngotchac commented Feb 3, 2017 • edited Loading

jacogr commented Feb 3, 2017

jacogr commented Feb 2, 2017 •

edited

Loading

ngotchac commented Feb 3, 2017 •

edited

Loading

ngotchac commented Feb 3, 2017 •

edited

Loading

ngotchac commented Feb 3, 2017 •

edited

Loading