Conversation
Signed-off-by: Onur Özkan <work@onurozkan.dev>
Signed-off-by: Onur Özkan <work@onurozkan.dev>
Signed-off-by: Onur Özkan <work@onurozkan.dev>
Signed-off-by: Onur Özkan <work@onurozkan.dev>
Signed-off-by: Onur Özkan <work@onurozkan.dev>
|
@onur-ozkan |
Signed-off-by: Onur Özkan <work@onurozkan.dev>
|
Should be fixed now, thanks. |
|
@cipig please take a look at this Pr and see if it matches what you needed |
shamardy
left a comment
There was a problem hiding this comment.
First review iteration from my side. I also have a question, what happens on restart, do we check expirations of saved orders before rebroadcasting them?
| rel_nota: Option<bool>, | ||
| #[serde(default = "get_true")] | ||
| save_in_history: bool, | ||
| timeout_in_minutes: Option<u16>, |
There was a problem hiding this comment.
I think using seconds would be unnecessary and can be bad for the network stability (imagine people sending orders with ~5-10 seconds of expiration time).
There was a problem hiding this comment.
Makes sense, will wait for @cipig input since he requested this
There was a problem hiding this comment.
imagine people sending orders with ~5-10 seconds of expiration time
On the other hand, if or when we can support this, it won't be bad as it will mirror the CEX experience a lot.
There was a problem hiding this comment.
And they can cancel it in whatever seconds they like manually or using scripts
There was a problem hiding this comment.
It wouldn't be harmful on CEX platforms because spamming very short-lived orders doesn't affect other nodes. But on a P2P network I think orders should live for at least 30 seconds or so. For example orders that last only 5 seconds just put pressure on the network, especially since it can take up to 5 seconds for peers to fully sync. Instead doing manual validations on the expiration time, I decided to make it minute and I tought it's quite reasonable to say "orders must have at least 1 minute expiration time" to users.
Also, when taking so many users into account, with seconds, this feature alone can put quite a lot of pressure on the network, with minutes, it should be way less compare to seconds.
There was a problem hiding this comment.
I understand and agree. I was just arguing the opposite case, I plan to leave it at minutes unless @cipig objects and wants something like 30 seconds.
There was a problem hiding this comment.
Cancelling in seconds doesn't have to have an immediate p2p message propagated but if it's matched before it's cancelled on remote nodes, the maker node will not proceed with the order when it's not available locally. And takers choose between the best response from multiple makers anyway so it won't cause problems. But that's for another PR or issue if needed.
There was a problem hiding this comment.
Makes sense, will wait for @cipig input since he requested this
Minutes is perfectly fine. I added it in my bot with a 20 minute timeout (cipig/mmtools@5ee5dbe). It needs anyway 5-15 minutes to update all orders, depending on number of coins. If the bot dies for whatever reason, orders are cancelled like this:
22 14:32:03, mm2_main::lp_ordermatch:3591] INFO Order 'afb833e8-7d6a-42da-9151-4f8f096efb57' is expired, cancelling
22 14:32:05, mm2_main::lp_ordermatch:3591] INFO Order 'f1cc74c7-b0f0-40d7-a4a6-036ea66a39e2' is expired, cancelling
22 14:32:07, mm2_main::lp_ordermatch:3591] INFO Order 'fc996c71-0d19-4e7c-bc35-636afdd88d22' is expired, cancelling
22 14:32:09, mm2_main::lp_ordermatch:3591] INFO Order '28088d79-694b-4b84-965b-1c97b0827729' is expired, cancelling
That's exactly what i wanted, as a safety measure for my own bad programming :-)
Thanks a lot.
… into expirable-orders
* dev: fix(eth): Correctly implement ETH max withdrawal logic (#2531) feat(use-clap-for-cli): use clap to parse CLI-Args #2215 (#2510) feat(orderbook): expirable maker orders (#2516) improvement(eth): drop parity support (#2527) chore(release): finalize changelog for v2.5.0-beta (#2524) chore(toolchain): upgrade toolchain to nightly 1.86.0 (#2444) feat(swap): rpc to find best swap with liquidity routing for ask (#2362) fix(kdf_walletconnect): apply explicit MmError mapping (#2514) fix(walletconnect): centralize connection and retry logic (#2508) fix(hw-wallet): avoid calling `get_enabled_address` in trezor-based coin init (#2504)
* refactor-gas-fee-policy: (30 commits) revert change in test_enable_custom_erc20 fix(test) disable get_swap_gas_fee_policy for tests (no platform coin in tests) revert tokio test and use block_on for eth docker test Refactor gas fee code: get max_eth_tx_type from platform coin, add gas fee adjust coin params, rename "set_swap_transaction_fee_policy" rpc to "set_swap_gas_fee_policy", force set_swap_gas_fee_policy to store policy in the platform coin fix(eth): Propagate structured EIP-1559 fee errors (GLEECBTC#2532) fix(eth): Correctly implement ETH max withdrawal logic (GLEECBTC#2531) feat(use-clap-for-cli): use clap to parse CLI-Args GLEECBTC#2215 (GLEECBTC#2510) feat(orderbook): expirable maker orders (GLEECBTC#2516) improvement(eth): drop parity support (GLEECBTC#2527) chore(release): finalize changelog for v2.5.0-beta (GLEECBTC#2524) chore(toolchain): upgrade toolchain to nightly 1.86.0 (GLEECBTC#2444) feat(swap): rpc to find best swap with liquidity routing for ask (GLEECBTC#2362) fix(kdf_walletconnect): apply explicit MmError mapping (GLEECBTC#2514) fix(walletconnect): centralize connection and retry logic (GLEECBTC#2508) fix(hw-wallet): avoid calling `get_enabled_address` in trezor-based coin init (GLEECBTC#2504) chore(core): replace hash_raw_entry with stable entry() API (GLEECBTC#2473) chore(core): adapt `MmError` and usages for compatibility with new rustc versions (GLEECBTC#2443) feat(wallet): add `delete_wallet` RPC (GLEECBTC#2497) chore(release): add changelog entries for v2.5.0-beta (GLEECBTC#2494) chore(release): bump kdf version to 2.5.0-beta (GLEECBTC#2492) ... # Conflicts: # mm2src/coins/eth.rs # mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs # mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs # mm2src/coins/eth/eth_utils.rs # mm2src/coins/eth/eth_withdraw.rs # mm2src/coins/eth/v2_activation.rs # mm2src/coins/lp_coins.rs # mm2src/coins/qrc20.rs # mm2src/coins/qrc20/swap.rs # mm2src/coins_activation/src/platform_coin_with_tokens.rs # mm2src/coins_activation/src/token.rs # mm2src/mm2_main/src/lp_swap/check_balance.rs # mm2src/mm2_main/src/lp_swap/maker_swap.rs # mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs # mm2src/mm2_main/src/rpc/lp_commands/ext_api/ext_api_types.rs # mm2src/mm2_main/src/rpc/lp_commands/mod.rs # mm2src/mm2_main/src/rpc/lp_commands/one_inch/errors.rs # mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs # mm2src/mm2_main/src/rpc/lp_commands/tokens.rs # mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs # mm2src/trading_api/src/one_inch_api/classic_swap_types.rs # mm2src/trading_api/src/one_inch_api/client.rs # mm2src/trading_api/src/one_inch_api/portfolio_types.rs
|
one last question: if not too much work, can we add more infos to the |
* dev: (21 commits) feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499) feat(utxo): add new fixed txfee option for DINGO-like coins (#2454) ci(pull-requests): review notification bot (#2468) improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538) bless clippy (#2560) refactor(toolchain): use latest available stable compiler (#2557) feat(wallet): implement unified offline private key export API (#2542) improve note for docker test start failure (#2550) fix(DOCS): add note for macos to fix docker containers startup failure (#2544) refactor(toolchain): general stabilization for stable rust (#2528) fix(ci): adds nodejs 20 to ci-container (#2536) fix(WASM and Debian): fix build failures (#2534) improvement(event-streaming): impl DeriveStreamerId trait for all streamers (#2489) fix(eth): Propagate structured EIP-1559 fee errors (#2532) fix(eth): Correctly implement ETH max withdrawal logic (#2531) feat(use-clap-for-cli): use clap to parse CLI-Args #2215 (#2510) feat(orderbook): expirable maker orders (#2516) improvement(eth): drop parity support (#2527) chore(release): finalize changelog for v2.5.0-beta (#2524) chore(toolchain): upgrade toolchain to nightly 1.86.0 (#2444) ... # Conflicts: # mm2src/coins/lp_coins.rs # mm2src/coins/rpc_command/get_new_address.rs # mm2src/trezor/src/eth/eth_command.rs
Implements expirable maker orders which can be easily used with timeout_in_minutes arg in setprice RPC. It's an optional field and doesn't cause a breaking-change.
Implements expirable maker orders which can be easily used with
timeout_in_minutesarg insetpriceRPC. It's an optional field and doesn't cause a breaking-change.Changes on the 1.5.0 timed-map release: Orkavian/timed-map#12
Documentation issue: GLEECBTC/komodo-docs-mdx#537
Closes #2479