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

Currency and CurrencyService #695

Merged
merged 11 commits into from Oct 26, 2020
Merged

Currency and CurrencyService #695

merged 11 commits into from Oct 26, 2020

Conversation

fboucquez
Copy link
Contributor

@fboucquez fboucquez commented Oct 19, 2020

Hi Steve, I've done a small refactoring to the NetworkCurrency object to align with Java.

NetworkCurrency is an object that knows how to create Mosaic (it's not a subclass of mosaic anymore). Now we can create NetworkCurrency based on the network configuration.

I've removed cat.currency and cat.harvest from the production code as these names are just generic that can be easily changed via bootstrap.

There is a NetworkCurrencyService that knows how to load/create NetworkCurrency objects. NetworkCurrency could also be loaded from user-provided mosaic id if desired.

Fixes #692
Fixes #693

@fboucquez fboucquez requested a review from rg911 October 19, 2020 20:59
Copy link
Contributor

@rg911 rg911 left a comment

Choose a reason for hiding this comment

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

Looks very good.
commented on some naming conventions and docs missing

src/service/interfaces/INetworkCurrencyService.ts Outdated Show resolved Hide resolved
src/service/interfaces/INetworkCurrencyService.ts Outdated Show resolved Hide resolved
src/service/interfaces/INetworkCurrencyService.ts Outdated Show resolved Hide resolved
@fboucquez
Copy link
Contributor Author

fboucquez commented Oct 19, 2020

@rg911 I've renamed NetworkCurrency with Currency. Currency is a generic object that creates mosaics. Putting the Network prefix was confusing for me as it could be a custom one.

They may be a network mosaic/alias or a custom/user mosaic/alias. NetworkCurrencies is still there because it holds the network currencies main and harvest. If you are happy I'll do the same in java.

I've also done the fixes as requested.

Docs will require a big update. All the hardcoded values in the typescript examples can be removed.

image

@fboucquez fboucquez requested a review from rg911 October 19, 2020 22:14
@fboucquez fboucquez changed the title Network Currency and Service Currency and CurrencyService Oct 19, 2020
@fboucquez fboucquez changed the base branch from main to dev October 24, 2020 11:19
@fboucquez
Copy link
Contributor Author

@rg911 could you review? This PR doesn't require a rest update like the next ones.

/**
* An object that knows how to create Mosaics based on the Mosaic Info and Namespace configuration.
*/
export class Currency {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we call it NetworkCurrency instead of Currency? to differentiate with other tokens might be created by users

Copy link
Contributor Author

@fboucquez fboucquez Oct 26, 2020

Choose a reason for hiding this comment

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

That's the thing, Currency supports user-created mosaics...

You can create user mosaic for transfers using the currency objects.

@rg911 rg911 merged commit 791bc93 into dev Oct 26, 2020
@fboucquez fboucquez deleted the network_currency branch October 26, 2020 14:43
rg911 added a commit that referenced this pull request Dec 8, 2020
* Updated Travis.
Using travis function submodule

* Fixed e2e script

* Currency and CurrencyService (#695)

* Network Currency and Service

* Code improvements

* Renamed NetworkCurrency with Currency
Docs fixes

* Removed cache from service. The repository factory caches them

* Fixed lazy loading cache

* Improved e2e for travis cron
Mosaic id is prefered when loading from rest.
Removed hardcoded mosaic id from public

* Fixed lint error

* Fixed build

* Updated bootstrap

* create-index-files npm command (#701)

Generates the index.ts automatically

* Revisit how npm package is created (#702)

* Updated open api to latest alpha (Requires New Rest) (#698)

* Fixed #672
- Added finalization http

* Security alert fix

* Updated libs

* Updated open api to latest alpha
Added secret filter in secret lock search
Updated transaction search

* Improved e2e tests

* Increased timeout

* Fixed build error

Co-authored-by: Steven Liu <xian.f.liu@gmail.com>

* Typo in docs of PublicAccount.ts (#703)

* UnlockedAccount endpoint and latest openAPI changes (#706)

* Fixed #705
- Added unlocked account endpoint
- Applied latest changes from openAPI

* Updated open api to latest alpha (Requires New Rest) (#698)

* Fixed #672
- Added finalization http

* Security alert fix

* Updated libs

* Updated open api to latest alpha
Added secret filter in secret lock search
Updated transaction search

* Improved e2e tests

* Increased timeout

* Fixed build error

Co-authored-by: Steven Liu <xian.f.liu@gmail.com>

* Fixed #705
- Added unlocked account endpoint
- Applied latest changes from openAPI

* Updated change log

Co-authored-by: fboucquez <fboucquez@gmail.com>

* Added prettier-plugin-organize-imports to prettier. (#708)

* Added prettier-plugin-organize-imports to prettier.
It fixes all the unsued imports warnings and order conflicts.
It's integrated with npm run style:fox

* Updated openAPI spec for unlockedAccount (#710)

* Updated openAPI spec for unlockedAccount

* Added Peer param in unlocked account

* Updated travis (#711)

* PersistentDelegationRequestTransaction message bug fix (#709)

* Fixed message parsing from dto and buffer

* State serialization and Mako Catbuffer (#707)

* message marker harvesting (#714)

* Fixed persistentHarvesting message marker issue.

* Working on persistent delegate harvest

* Improved doc and unit tests

Co-authored-by: fernando <fboucquez@gmail.com>

* - Added nodePublicKey in NodeInfo (#719)

- Cache nodePublicKey in RepoFactory

* /Merkle endpoints (#717)

* Added /merkle endpoints

* Added new block type (#724)

* Added new block type

* PR feedbacks

* Server 10.0.0.4 upgrade (#725)

* V1 Transactions

* Added voting key v1 and v2

* fixed lint

* Fixed DTO mapping and improved unit test.

* Fixed repository merkle name
Fixed search criteria

* Server duration parser and test refactor (#729)

* - Fixed #728
- server duration parser and test refactor

* Added more check in duration parser

* All transaction related notification should take an unresolved address (#731)

* issue-#664: change filter Address to be unresolved Address

* [issue-664]: resolve transaction notification with unresolved address

* [issue-664]: fix invalid address name in docs

* [issue-664]: add some test case for unresolved address

* Merkle proof + finalization proof + block type fixes (#727)

Improved creation of streamers
Finality proof fix
Make nemesis/importance block optional

Co-authored-by: fernando <fboucquez@gmail.com>

* Fixed deadline is sometimes 1 sec different

* instead of minus milsecs, minus secs, sinc server epochAdjustment is using secs

* New Role types (#733)

* Bump highlight.js from 10.1.2 to 10.4.1 (#735)

Bumps [highlight.js](https://github.com/highlightjs/highlight.js) from 10.1.2 to 10.4.1.
- [Release notes](https://github.com/highlightjs/highlight.js/releases)
- [Changelog](https://github.com/highlightjs/highlight.js/blob/master/CHANGES.md)
- [Commits](highlightjs/highlight.js@10.1.2...10.4.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* 0.22.0 changelog (#732)

* Updated version to 0.22.0
Updated changelog

* Feedback changes

* State proof fixes (#736)

* Added version 1 fallback

* removed log

* Addes sort_key:keys (#737)

* Updated change log (#738)

* Version upgrades (#739)

* Version upgrades

* updated open api version

Co-authored-by: Steven Liu <xian.f.liu@gmail.com>
Co-authored-by: Xavi Artigas <xavierartigas@yahoo.es>
Co-authored-by: James Lin <54219053+james-lin71@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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