Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Prevent getFeesAsync method on HttpClient from mutating input #296

Merged
merged 3 commits into from
Jan 9, 2018

Conversation

BMillman19
Copy link
Contributor

This PR:

  • Remove type conversion of the feesRequest in getFeesAsync()
  • JSON.stringify in the _reqestAsync() method already takes care of this for us
  • Remove BigNumber to string type conversion utility because it is no longer used
  • Added a test

Copy link
Contributor

@fabioberger fabioberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure there is no mutation in any other public methods. Then, LGTM.

const takerTokenAmountBefore = new BigNumber(request.takerTokenAmount);
const saltBefore = new BigNumber(request.salt);
const expirationUnixTimestampSecBefore = new BigNumber(request.expirationUnixTimestampSec);
await relayerClient.getFeesAsync(request);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure there is no mutation in any of the other methods in Connect?

@fabioberger
Copy link
Contributor

@BMillman19 don't forget to add a CHANGELOG entry for Connect. It should be part of this PR.

* development:
  Changes to abi-gen after code review
  Added constructor ABIs to abi-gen
  Describe #295 in a CHANGELOG
  Add #302 description to changelog
  Fix formatting
  Apply prettier config
  Install prettier
  Remove formatting esilnt rules
  sendTransactionAsync should return txHash string
  Added Event generation to abi-gen
  Publish
  Add dates to CHANGELOG entries
  Update subproviders CHANGELOG
  Support both personal_sign and eth_sign
  Fix Ledger tests given change from `personal_sign` to `eth_sign`
  Update subprovider to catch correct RPC method
  Rename guide
  Update contribution guide
  Fix broken links in the abi-gen README
  Fix typing generation for arrays in which types separated by |s
@BMillman19 BMillman19 merged commit 7a56e83 into development Jan 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants