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

add KSM (Kusama) support #109

Merged
merged 5 commits into from
Jun 25, 2024
Merged

add KSM (Kusama) support #109

merged 5 commits into from
Jun 25, 2024

Conversation

goulinkh
Copy link
Contributor

@goulinkh goulinkh commented Jun 24, 2024

No description provided.

@LeTamanoir LeTamanoir self-requested a review June 24, 2024 15:20
async decodeTx(txSerialized: string): Promise<UnsignedTransaction> {
const { data } = await api.get<UnsignedTransaction>(`/v1/dot/transaction/decode?tx_serialized=${txSerialized}`);
return data;
mainToPlanck(amount: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

why name it "generically" if it does not do the same for KSM and DOT ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this function has the same role for all substrate tokens, converting from the main token to planck however the conversion rate is different for DOT, WND & KSM hence the abstract keyword. Does this not look straight forward to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

for all other protocols we have a naming with token names no ?

Copy link
Contributor

Choose a reason for hiding this comment

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

for consistency I guess we should have ksmToPlanck and dotToPlanck no ?

Copy link
Contributor Author

@goulinkh goulinkh Jun 24, 2024

Choose a reason for hiding this comment

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

Ok let me find a workaround. Let me know if this looks good for you

@goulinkh goulinkh merged commit 9a64873 into main Jun 25, 2024
1 check passed
@goulinkh goulinkh deleted the add-ksm-support branch June 25, 2024 07:35
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.

2 participants