Skip to content

Commit

Permalink
Add PACcoin ($PAC) currency (#408)
Browse files Browse the repository at this point in the history 
Taken from: jl777/coins@57ac3e2

https://coinmarketcap.com/currencies/paccoin/
  • Loading branch information
@sindresorhus
sindresorhus committed Jul 12, 2018
1 parent 478bb91 commit 31d55a7
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
18 changes: 18 additions & 0 deletions app/marketmaker/supported-currencies.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ We use the `name` property only when the currency is not on `coinmarketcap.com`.
*/

const supportedCurrencies = [
{
coin: '$PAC',
rpcport: 7111,
pubtype: 55,
p2shtype: 10,
wiftype: 204,
txfee: 10000,
electrumServers: [
{
host: 'electrum.paccoin.io',
port: 50001,
},
{
host: 'electro-pac.paccoin.io',
port: 50001,
},
],
},
{
coin: 'ARG',
rpcport: 13581,
Expand Down
2 changes: 1 addition & 1 deletion app/renderer/api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import MarketmakerSocket from './marketmaker-socket';

const getPort = electron.remote.require('get-port');

const symbolPredicate = ow.string.alphanumeric.uppercase;
const symbolPredicate = ow.string.uppercase;
const uuidPredicate = ow.string.alphanumeric.lowercase;

const errorWithObject = (message, object) => new Error(`${message}:\n${util.format(object)}`);
Expand Down

8 comments on commit 31d55a7

@cipig
Copy link

@cipig cipig commented on 31d55a7 Jul 22, 2018

Choose a reason for hiding this comment

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

the symbol is "PAC": https://github.com/cipig/coins/blob/master/coins#L51

@sindresorhus
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not according to CMC https://coinmarketcap.com/currencies/paccoin/

@cipig
Copy link

@cipig cipig commented on 31d55a7 Jul 22, 2018

Choose a reason for hiding this comment

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

Half of the exchanges also use PAC as symbol, it is PAC in jl's repo and i use it on my LPs too. $ is a special character and using it can cause problems in scripts.

@sindresorhus
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't feel strongly about it, but it's kinda weird to not use the official symbol name. We would also have to do a workaround since our CMC fetching would not match PAC. @lukechilds Thoughts?

@cipig
Copy link

@cipig cipig commented on 31d55a7 Jul 22, 2018

Choose a reason for hiding this comment

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

autoprice call in mm (which uses CMC v1 API) uses part of the URL to identify the coins on CMC, not the coin symbols. CMC v2 API and CMC paid API use ids to identify the coins. I have collected this ids: https://github.com/cipig/mmtools/blob/master/setprices/margins.conf and use them for my own price update script for CMC paid API.

@lukechilds
Copy link
Member

@lukechilds lukechilds commented on 31d55a7 Jul 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@cipig If we use a different coin name does that mean we won't be able to match with your LPs?

If so, we should always use whatever is in the jl777/coins repo to ensure we match with the rest of the network.

@sindresorhus I know it's a bit dirty but we could add a symbol property to the coin object and use that for all visual stuff but use coin for all mm related stuff. That way the user always see's the ticker they expect.

I already considered implementing that to fix: #289

@cipig
Copy link

@cipig cipig commented on 31d55a7 Jul 22, 2018

Choose a reason for hiding this comment

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

Yes, the same coin symbols must be used across all nodes, else it won't match. We should use the same symbols across all apps. Problem is that coin symbols are not consistent on exchanges/CMC, eg look up BTM on CMC (it is used twice). I would recommend using other things to uniquely identify a coin, eg the id on CMC.

@lukechilds
Copy link
Member

Choose a reason for hiding this comment

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

Ok, thanks for the info @cipig.

@sindresorhus yeah I definitely think we should do that then.

Please sign in to comment.