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.

Sign up for GitHub

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

Jump to bottom

Extend countDecimals() for negative values #154

Merged
merged 3 commits into from
Oct 26, 2023
Merged

Extend countDecimals() for negative values #154

merged 3 commits into from
Oct 26, 2023

Conversation

platfowner
Copy link
Member

Change summary:

  • Extend countDecimals() for negative values
  • Change some comments and names

Related issues:

@platfowner platfowner added the enhancement New feature or request label Oct 25, 2023
@platfowner platfowner changed the title Feature/platfowner/feature Extend countDecimals() for negative values Oct 25, 2023
woomurf
woomurf approved these changes Oct 26, 2023
View reviewed changes
Copy link
Member

@woomurf woomurf left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

shyun-comcom
shyun-comcom reviewed Oct 26, 2023
View reviewed changes
src/wallet.ts
@@ -2,8 +2,8 @@ import { Accounts, Account, TransactionBody, V3Keystore, V3KeystoreOptions } fro
import Ain from './ain';
import { validateMnemonic, mnemonicToSeedSync } from 'bip39';
import Reference from './ain-db/ref';
const AIN_HD_DERIVATION_PATH = "m/44'/412'/0'/0/"; /* default wallet address for AIN */
const MAX_TRANSFERABLE_DECIMALS = 6; /* The maximum transferable decimal places of values */
const AIN_HD_DERIVATION_PATH = "m/44'/412'/0'/0/"; // The hardware wallet derivation path of AIN
Copy link
Member

Choose a reason for hiding this comment

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

저희 AIN wallet이나 aFan wallet에서는 이제 m/44'/60'/0'/0/을 derivation path prefix로 사용하기로 했는데, 여기서는 이 path로 유지하는게 맞을까요, 아니면 변경하는게 맞을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, just added a todo.
One problem is that we already have some v3 wallet files.
Thanks!

@platfowner
Copy link
Member Author

Thanks for the review! @shyun-comcom @woomurf Now merging..
Please let me know if you have any other comments.

@platfowner platfowner merged commit 3a0b9da into develop Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shyun-comcom shyun-comcom shyun-comcom left review comments

@woomurf woomurf woomurf approved these changes

@minsulee2 minsulee2 Awaiting requested review from minsulee2

@jiyoung-an jiyoung-an Awaiting requested review from jiyoung-an

@minjaeli minjaeli Awaiting requested review from minjaeli

@YunsuBae YunsuBae Awaiting requested review from YunsuBae

@akastercomcom akastercomcom Awaiting requested review from akastercomcom

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@platfowner @woomurf @shyun-comcom