e-money / em-ledger Goto Github PK

Describe the bug
Code at

.. does not take into consideration that there may exist multiple orders for a unique (src, dst).
The sum of SourceRemaining for the same (src, dst) should not exceed the available account balance.

The current implementation does not handle this case, but only deals with it on order creation:

How to reproduce

• With an account balance of 10 eEUR.
• Create 10 orders selling 1 eEUR for e.g. 1 eCHF (src: eEUR, dst: eCHF)
• Send 7 eEUR from the account, reducing the balance to 3 eEUR.
• 10 orders will remain with unchanged sourceRemaining, as the 3 eEUR balance is still above 1 eEUR for each individual order.

Expected behavior
Invariant: The sum of the sourceRemaining for identical (src, dst) orders should not exceed the spendable account balance.

Working through the orders in reverse distance from the best price, i.e. "orders the farthest away from the best price first", should have they sourceRemaining reduced and canceled until the above invariant holds.

Penalties from slashing should be burned rather than sent to the distribution module. Distributing them is legacy behaviour from a previous token model.

Move the functionality in hooks/distribution to x/distribution.

Describe the bug
When starting emd with the --halt-height flag, the node cannot later continue participating in consensus.

Killing the node with ctrl+c does not yield this problem.

How to reproduce

1. Start full node with seeds and add the flag --halt-height 100.
2. The node will synchronize and shut down at block 100.
3. Start the node again without a halt height.
4. The node will stop and display the error message panic: state.AppHash does not match AppHash after replay.

Expected behavior
The node should continue synchronizing from block 100.

For a smoother upgrade experience, lets us https://github.com/cosmos/cosmos-sdk/tree/master/x/upgrade

denom_utils.go contains ValidateDenom which now seems like an unneeded extra abstraction. Remove it.

The present day value of reserve balances should be moved on-chain to avoid having an external dependency for exchange rate and reserve data.

As discussed with @blewater we could consider using existing oracle solutions to feed this information? It would be a setup with a fairly low number of witnesses managed by e-Money A/S that connect with the banking APIs. But certainly more than one for redundancy.

Ideally the solution would incorporate the "stablecoin registry" level of details described here:
https://github.com/e-money/em-ledger/blob/master/docs/tokens.md

Tasks:

• Research suitable oracle solution (if any)
• Add reserve balances to issuer
• Let issuer in/decrease reserve balances through transactions (or oracle)
• External: Create witness process that can interact with bank APIs and update information on-chain
• ...
• Add queries for exchange rate
• Add queries for reserve information

Describe the bug
Submitting a single tx results in 3 consecutive blocks created.

How to reproduce
Submit tx on v0.5.5

Expected behavior
2 blocks: one containing the tx and one for finalisation.

Screenshots or log output

Jan 19 13:03:56 i-0f6670abb7a863172 emd[1647]: I[2020-01-19|13:03:56.874] Executed block                               module=state height=10 validTxs=1 invalidTxs=0
Jan 19 13:03:56 i-0f6670abb7a863172 emd[1647]: I[2020-01-19|13:03:56.876] Committed state                              module=state height=10 txs=1 appHash=6D2FF0A609BD0AEE312ACFD282FF7FA3C6647F6396F397DA9044CA92C0501C8B
Jan 19 13:03:57 i-0f6670abb7a863172 emd[1647]: I[2020-01-19|13:03:57.381] Executed block                               module=state height=11 validTxs=0 invalidTxs=0
Jan 19 13:03:57 i-0f6670abb7a863172 emd[1647]: I[2020-01-19|13:03:57.383] Committed state                              module=state height=11 txs=0 appHash=9B833C0F3E03BD4E7F4917B8E8409713822B6DB87389FBE78DBB7E324FD877C4
Jan 19 13:03:57 i-0f6670abb7a863172 emd[1647]: I[2020-01-19|13:03:57.890] Executed block                               module=state height=12 validTxs=0 invalidTxs=0
Jan 19 13:03:57 i-0f6670abb7a863172 emd[1647]: I[2020-01-19|13:03:57.892] Committed state                              module=state height=12 txs=0 appHash=9B833C0F3E03BD4E7F4917B8E8409713822B6DB87389FBE78DBB7E324FD877C4

Additional context
Related to changes in inflation gating?

Describe the bug
Transfer that fails due to being a restricted denomination, is reported as succesful.

How to reproduce
The failed transfer at https://e-money.network/transactions/38CEF4E9AFB7FFC3113ED146465A7F5208174B2D4A483E2F3BE22ECDDEE0BA15

is reported as succesful by the light client interface:
http://emoney.validator.network/light/txs/F2C3698A886B96ED48A8CBBB3452CD5035B51BBC7E3BF86662A00AE4A2CFB4AF

For display purposes in the wallet UI, it would be great to have a "created" timestamp for market orders, based on the block time when it was submitted.

The "created" timestamp should be returned through the queries "market/account" and "market/instrument//"

Describe the bug

% emcli query authority gas-prices
 {"min_gas_prices":"0.400000000000000000eeur,0.440000000000000000eusd,0.800000000000000000ungm"}"

Expected behavior
Should default to "text" output.

Screenshots or log output
Notice --output below:

% emcli query authority gas-prices --help
Query the current minimum gas prices

Usage:
  emcli query authority gas-prices [flags]

Flags:
      --height int    Use a specific height to query state at (this can error if the node is pruning state)
  -h, --help          help for gas-prices
      --indent        Add indent to JSON response
      --ledger        Use a connected Ledger device
      --node string   <host>:<port> to Tendermint RPC interface for this chain (default "tcp://localhost:26657")
      --trust-node    Trust connected full node (don't verify proofs for responses)

Global Flags:
      --chain-id string   Chain ID of tendermint node
  -e, --encoding string   Binary encoding (hex|b64|btc) (default "hex")
      --home string       directory for config and data (default "/Users/mdyring/.emcli")
  -o, --output string     Output format (text|json) (default "text")
      --trace             print out full stack trace on errors

It would be great to add some additional information in the instruments query:
https://emoney.validator.network/light/market/instruments

{
  "height": "82308",
  "result": {
    "instruments": [
      {
        "source": "eeur",
        "destination": "ungm",
        "last_price": "1.538399692959211348",
        "last_traded": "2020-11-04T18:12:40.400949361Z"
      },
      {
        "source": "ungm",
        "destination": "eeur",
        "last_price": "0.650026130775179302",
        "last_traded": "2020-11-04T18:12:40.400949361Z"
      }
    ]
  }
}

We should add a "best_price" value that is price returned by invoking createExecutionPlan.

It seems there are leftovers:
http://lilmermaid.validator.network/light/distribution/community_pool

Describe the bug
Community pool is not completely removed. Any remainder that may exist when paying out rewards is sent to the community pool, which will thus slowly accumulate a small balance over time.

How to reproduce
The current balance of the community pool can be inspected using the light client interface, e.g. https://lilmermaid.validator.network/light/distribution/community_pool

The addition of funds to its balance takes place in several places, notably withdrawDelegationRewards method in delegation.go in the distribution module.

Expected behavior
Community fund should ideally be completely disabled. A possible solution would be to periodically add its balance to the distribution account, once the balance is large enough to be able to do it.

This is now possible on Cosmos SDK master branch, let's wait until it makes it into a release.

Investigate whether Liquidity provider accounts are correctly read from genesis files.

We're currently using a rather cumbersome approach to validating denominations. An SDK approach has been created and should be used instead.

Instead of using the last traded price, the buyback module should use the best passive price (using bestPrice).

This is safer and avoids the need of "bootstrapping" the market with a trade before buyback can start.

Placeholder issue. Should most likely live outside of em-ledger.

Investigate and determine which standard payment APIs to support:

• https://www.w3.org/TR/payment-request/
• https://w3c.github.io/webpayments/proposals/interledger/
• https://payid.org

"Payment Request API is basically for card payments, but Interledger is an alternative payment method. Ie., a user can choose Interledger instead of Visa or Mastercard when selecting a payment method, and then gets redirected to an ILP wallet. Interledger interfaces with a paradigm that is equivalent to Crypto-Conditions, as described under 3.2 of the Interledger payment draft. For Crypto-Conditions there is another standard https://tools.ietf.org/html/draft-thomas-crypto-conditions-04."

Using the cli to add a a market order with a non-existant denomination results in the rather unhelpful error response "Internal".

How to reproduce

1. Start a local testnet.
2. Add a new market order:
   ./build/emcli tx market add-limit 1ungmn 500bobs orderid1 --from <address>

The non-existant denomination "bobs" will make the tx fail. The output from the cli is:

codespace: market
code: 1
data: ""
rawlog: internal
logs: []
info: ""
gaswanted: 200000
gasused: 62176
tx: null
timestamp: ""

Expected behavior
The log should contain an error message informing of the erroneous denomination.

The concept of a "restricted denom" is no longer necessary and should be removed.

Add integration test to ensure the fixed gas prices work as expected, e.g. 25000 gas for add order.

Describe the bug
emcli does not set time_in-force for cancel

How to reproduce
emcli tx market cancelreplace order1 1000000000ungm 500000000eeur order2 --from <key> --gas-prices "1.0ungm"

Expected behavior
Time In Force should be handled identically to emcli market add-limit.

Screenshots or log output

{"chain_id":"lilmermaid-8","account_number":"71","sequence":"209","fee":{"amount":[{"denom":"ungm","amount":"200000"}],"gas":"200000"},"msgs":[{"type":"e-money/MsgCancelReplaceLimitOrder","value":{"owner":"emoney1uae5c48qjdc9psfzkwvre0shm9z8wlsfnse2nz","original_client_order_id":"order1","new_client_order_id":"order2","source":{"denom":"ungm","amount":"1000000000"},"destination":{"denom":"eeur","amount":"500000000"}}}],"memo":""}

confirm transaction before signing and broadcasting [y/N]: y
Enter keyring passphrase:
height: 106
txhash: 80444FF8E3E5944B6882A0FB54D944A108C9F23C23E72AED0C2255EAE2F6614C
codespace: market
code: 12
data: ""
rawlog: ': Unknown ''time in force'' specified : <unknown>: failed to execute message; message index: 0'
logs: []
info: ""
gaswanted: 200000
gasused: 49759
tx: null
timestamp: ""

Gas prices for the network is currently controlled by the authority account, and enforced by the standard implementation of the em-ledger software.

It's possible for a validator to run a version of the em-ledger software that ignores the gas prices set by the authority account. There is currently no penalty for this.

This behaviour can be turned into a network offense, that could lead to either jailing or slashing. Other nodes could inspect committed blocks to see if the transaction fees were enforced. It should be investigated whether Tendermint's evidence and byzantine validator mechanisms can be used.

Check whether dependency of the liquidity provider module can be reduced to the bank module.

Is your feature request related to a problem? Please describe.
A misbehaving validator could modify em-ledger code to ignore the centralised gas prices.

Describe the solution you'd like
A block proposer should be slashed if it includes transactions with too low fees (compared to the central gas-prices).

Add market orders:
Does not include a limit price. Instead this is calculated dynamically by using the last traded bruce and a slippage value (default of, say, 1%).

Add "time in force":
GTC - Good Til Canceled (Default for limit orders)
IOC - Immediate Or Cancel (Default for market orders)
FOK - Fill Or Kill (Useful for payments in that include a mandatory conversion)

Is your feature request related to a problem? Please describe.
To incentivise market makers, net adding liquidity over time should be free. This can be unpacked as:

• Adding liquidity should be free, e.g. MsgAddLimitOrder and potentially MsgAddMarketOrder.
• Removing liquidity should never be free, e.g. MsgCancelOrder.
• Replacing liquidity should be free, but only if not done too often (to avoid spamming the chain).

Describe the solution you'd like

1. Make the gas associated with all market messages identical, so that MsgCancelOrder potentially carries same cost as MsgCancelReplaceLimitOrder.
2. New orders that add liquidity should receive a gas rebate proportional to how much of the order was filled already (0% fill means 100% rebate):
   

   em-ledger/x/market/keeper/keeper.go

   Line 334 in 12b0302

   if addToBook {

3. Canceled orders always pay full gas.
4. Replaced orders are only eligible for the gas rebate in 2. if they are replacing an order older than, say, 5 minutes. This requires #34 to work.

Additional considerations
It must not be possible to exploit the rebate mechanism in a way where it becomes cheaper (or even free) to remove liquidity, i.e. crafting an order to get almost filled but adding a small amount to the book should not result in a rebate.

It must not be possible to spam the chain with useless transactions. Only behaviour that is net adding liquidity

Describe alternatives you've considered
Further investigation needed here.

Users should only pay for gas consumed on a transaction, not the maximum allowance.

This is to prevent "fat finger" incidents (as seen on Cosmos Hub) and for fairness.

This seems in line with what IRISnet is doing: https://github.com/irisnet/irishub/blob/fdc78914a514ead0fc62d4773dcbae1a31b66fac/app/v1/auth/feeauth.go#L63

MaxSlippage is ignored when handling

MsgCancelReplaceOrder struct {
	Owner             sdk.AccAddress `json:"owner" yaml:"owner"`
	Source            sdk.Coin       `json:"source" yaml:"source"`
	Destination       sdk.Coin       `json:"destination" yaml:"destination"`
	OrigClientOrderId string         `json:"original_client_order_id" yaml:"original_client_order_id"`
	NewClientOrderId  string         `json:"new_client_order_id" yaml:"new_client_order_id"`
	MaxSlippage       sdk.Dec        `json:"maximum_slippage" yaml:"maximum_slippage"`
}

We should offer two distinct MsgCancelReplaceOrder messages, one for limit orders and one for market orders.

The Cosmos Stargate SDK upgrade introduced Protobuf serialization and gRPC as the API interface. Even the revamped Rest endpoints are served through the gRPC gateway pipe and lose their simplicity benefits when signing transactions. 
While the code base features Gingko integration tests, these exercise the CLI interface hardly the nominal choice for integrating external applications.

Exceptions

1. A POC demonstrating legacy transaction signing compatibility with the Ledger device through the CosmJS library.

2. A gRPC client with minimal API surface for query/response and transaction signing layer for the Bep3 module.

Proposed Solutions

• Replace a good portion of the existing Gingko tests with the CosmJS library that uses Protobuf and gRPC in its Stargate edition the defacto standard for external application integration work. Moreover, the same tests would function as an on-boarding aid and functional code samples for front-end integration work.

• Test with an enhanced gRPC Client. An obvious benefit is a tight integration with the existing Go source base and rapid implementation speed. They may also form the basis for a future em-ledger SDK for Golang integrators, though unlikely a popular choice for the envisioned audiences.

Not sure where this breaks in the stack, but inserting a "&" in the memo on wallet.e-money.com leads to an "invalid signature error".

Reproducible with Ledger device, both directly connected to wallet but also through Keplr.

One way to do this would be introduce a "Merchant" account type, that will reimburse transaction fees for payments sent to the account.

Could be more generalised as a "collect call" mechanism... Needs to be explored in further detail. Might some prior issues on cosmos-sdk repo that we can use as background research.

emcli contains a command for voluntarily funding the community pool: emcli tx distribution fund-community-pool

The option to do this from the client should be removed. In addition to this, we should consider removing the handling of that particular transaction from the em-ledger software.

Example:

{"Owner":"emoney1wq6eyug6njz9yqdn0jhsxrvm7x8v3v2zfkzk8s","Source":{"denom":"eeur","amount":"471096868463"},"Destination":{"denom":"eusd","amount":"500182000000"},"ClientOrderId":"clearthebook1"}

Should use snake case:

{"owner":"emoney1wq6eyug6njz9yqdn0jhsxrvm7x8v3v2zfkzk8s","source":{"denom":"eeur","amount":"471096868463"},"destination":{"denom":"eusd","amount":"500182000000"},"client_order_id":"clearthebook1"}

Describe the bug
After a multi-hour chain halt, any validator who is not signing the first block at recovery will be jailed. This is undesirable as many of these might be trying to recover and will now have to wait for the minimum jailing duration.

Expected behavior
Open for discussion. But:

• If there is more than (say) 1H between two blocks, it could make sense to reset the "missed block" state.
• Consider lowering minimum jailing duration as well.

Placeholder issue. Should most likely live outside of em-ledger.

We should strive to get the market API integrated with crypto trading libraries and products such as:

Trading products:

• https://hummingbot.io/

Libraries:

• https://github.com/ccxt/ccxt

API as a service:

• https://developers.shrimpy.io/
• https://www.coinapi.io/

Since a fixed gas price is charged for orders, the client order id should be capped to (say) 64 characters to avoid spamming.

Describe the bug

A validator encountered a crash of their validator and a sentry node.

How to reproduce

Have not attempted reproduction yet. Overall error message is

version 167924 was already saved to different hash 1DDC66CEFE812B7841376329476D57487E472E334906FBF7B9BCF940ED7A8A43 (existing hash 7B9226546B8D49856560E36600F99175E451E2F9182D53D367F32C95DD5A51C2)

Full stacktrace log is attached.

The issue happened at height: 167924

emd version --long
name: e-money
server_name: emd
client_name: emcli
version: 0.6.2
commit: ed08ab19e5f28649839069c44c967fe3e932c5b5
build_tags: netgo,ledger
go: go version go1.14.1 linux/amd64

The bug re-appeared when the validator did a re-sync from block 150.000, but disappeared when the priv_validator.json file was removed prior to re-sync.

emd_log.txt

Stopping emd during sync leads to data corruption. So far only reproducible on Linux, not MacOS.

Syncing:

...
I[2019-12-28|12:41:25.327] Committed state                              module=state height=586 txs=0 appHash=39B90E4E3358A5639BC84FE39BCC2EC280EABC191EFDD0AD81EAC707A460690A
I[2019-12-28|12:41:25.339] Endblock: Block 587 was proposed by D48BB5DA468ED61E27E52414CD11BE67983D128A module=emz 
I[2019-12-28|12:41:25.340] Executed block                               module=state height=587 validTxs=0 invalidTxs=0
E[2019-12-28|12:41:25.342] Connection failed @ recvRoutine (reading byte) module=p2p [email protected]:26656 conn=MConn{167.86.70.194:26656} err="read tcp 116.202.108.95:39516->167.86.70.194:26656: use of closed network connection"
E[2019-12-28|12:41:25.342] Stopping peer for error                      module=p2p peer="Peer{MConn{167.86.70.194:26656} 277ead1312fc456d98639b3e4fc8c37dc4df27c8 out}" err="read tcp 116.202.108.95:39516->167.86.70.194:26656: use of closed network connection"
E[2019-12-28|12:41:25.343] Not stopping ConsensusState -- have not been started yet module=consensus impl=ConsensusState
I[2019-12-28|12:41:25.343] exiting...                                   module=main

Crash:

emd start
I[2019-12-28|12:37:02.584] starting ABCI with Tendermint                module=main 
panic: failed to load Store: wanted to load target 525 but only found up to 0

goroutine 1 [running]:
emoney.NewApp(0x1555fe0, 0xc000a0caa0, 0x1567e20, 0xc000010230, 0xc0000aff00, 0x4)
        /home/emoney/em-ledger/app.go:192 +0x438b
main.newAppCreator.func1(0x1555fe0, 0xc000a0caa0, 0x1567e20, 0xc000010230, 0x0, 0x0, 0xc0009f5080, 0x9c)
        /home/emoney/em-ledger/cmd/emd/main.go:63 +0x52
github.com/cosmos/cosmos-sdk/server.startInProcess(0xc0000aff00, 0xc0003591e0, 0x1d, 0x0, 0x0)
        /home/emoney/go/pkg/mod/github.com/cosmos/[email protected]/server/start.go:142 +0x134
github.com/cosmos/cosmos-sdk/server.StartCmd.func1(0xc00046e280, 0x1fac4d0, 0x0, 0x0, 0x0, 0x0)
        /home/emoney/go/pkg/mod/github.com/cosmos/[email protected]/server/start.go:65 +0xb4
github.com/spf13/cobra.(*Command).execute(0xc00046e280, 0x1fac4d0, 0x0, 0x0, 0xc00046e280, 0x1fac4d0)
        /home/emoney/go/pkg/mod/github.com/spf13/[email protected]/command.go:826 +0x460
github.com/spf13/cobra.(*Command).ExecuteC(0xc000132000, 0x2, 0xc0008661a0, 0x11ba06d)
        /home/emoney/go/pkg/mod/github.com/spf13/[email protected]/command.go:914 +0x2fb
github.com/spf13/cobra.(*Command).Execute(...)
        /home/emoney/go/pkg/mod/github.com/spf13/[email protected]/command.go:864
github.com/tendermint/tendermint/libs/cli.Executor.Execute(0xc000132000, 0x1390330, 0x3, 0xc0008d7a80)
        /home/emoney/go/pkg/mod/github.com/tendermint/[email protected]/libs/cli/setup.go:89 +0x3c
main.main()
        /home/emoney/em-ledger/cmd/emd/main.go:55 +0x398

Describe the bug

The slashing module jails validators based on a ratio of missed to signed blocks due to the dynamic blocktime of the chain.

This creates a special case in the initial life of a chain where missing a low number of blocks results in a very large "missed ratio".

How to reproduce

Spin up a testnet and let one of the validators miss the first block. This will be interpreted as a "100% miss rate" and the validator will be immediately jailed.

Expected behavior

Allow the validators to miss a reasonable amount of blocks without getting slashed.

One way to solve this is to not jail a validator if blockCount in the slashing module's keeper is less than e.g. 100.

Summery

Very slow or not loaded when querying "validatorsets/latest" on rest-server

Network

lilmermaid-3

Steps to Reproduce

1. curl localhost:3317/node_info
   -> Quick results

2. curl localhost:3317/validatorsets/7000
   -> Quick results

3. curl localhost:3317/validatorsets/latest
   -> Slow results / No results(curl: (52) Empty reply from server)
   -> rest-server log: duration=42569

Is your feature request related to a problem? Please describe.
Collected transaction fees are distributed as staking rewards, leading to very small amounts (almost "dust") of stablecoins being distributed.

Describe the solution you'd like
Similar to stablecoin inflation, transaction fees should be sent to the buyback module instead of being distributed.

Describe alternatives you've considered
Burning of collected fee tokens. While it could make sense for stablecoins (reducing circulating supply), this would be foolish if external tokens (such as ATOMs) were accepted as fees later on.

Change "remaining" below to "source_remaining" and remove the denom.

{"height":"68503","result":{"source":"eeur","destination":"echf","orders":[{"id":10,"created":"2020-02-11T12:23:49.780587878Z","owner":"emoney107k6rxj7jy5xkupxdywnw803p8zg7js8580wut","remaining":"400000000eeur","price":"1.075000000000000000"}]}}