Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sendTransaction in ERC20 provider #182

Merged
merged 2 commits into from May 17, 2019
Merged

Conversation

harshjv
Copy link
Member

@harshjv harshjv commented May 15, 2019

  • Fix sendTransaction in ERC20 provider

@@ -4,8 +4,8 @@ import { BigNumber } from 'bignumber.js'
import Provider from '@liquality/provider'
import { padHexStart } from '@liquality/crypto'
import {
ensureAddressStandardFormat,
ensureHexEthFormat
toLowerCaseWithout0x,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea this is kind of what I meant when i said the naming might start to mean more than just the 0x 😄
I hope we don't start to have sentences describing what a function does eventually... This is why calling it just standard or normalised is a better naming in my opinion.

ensureAddressStandardFormat,
ensureHexEthFormat
toLowerCaseWithout0x,
ensure0x
Copy link
Collaborator

@monokh monokh May 15, 2019

Choose a reason for hiding this comment

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

Note that eventually this might not even be just 0x, but also [EIP-55] (https://github.com/ethereum/EIPs/blob/master/EIPS/eip-55.md) in which case we need to update these names again. Just another reason i think giving it a name rather than describing it is better

@harshjv harshjv merged commit 588eb81 into dev May 17, 2019
@kraikov kraikov deleted the fix-mm-erc20-sendtransaction branch April 29, 2022 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants