Skip to content

feat(stats_swaps): add gui version in status db#2061

Merged
shamardy merged 1 commit intodevfrom
feat-gui-version-in-db
Feb 15, 2024
Merged

feat(stats_swaps): add gui version in status db#2061
shamardy merged 1 commit intodevfrom
feat-gui-version-in-db

Conversation

@mariocynicys
Copy link
Copy Markdown
Collaborator

This PR aims to store the gui and mm_version data in the stats_swaps table.

Since gui and mm_version isn't communicated in swap messages, we only store our gui and version.
Seed nodes are able to store both maker and taker's gui and version since they get both.

fixes #2047

shamardy
shamardy previously approved these changes Feb 8, 2024
Copy link
Copy Markdown
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

LGTM! Only a few nits!

Comment on lines +112 to +114
fn migration_11() -> Vec<(&'static str, Vec<String>)> {
db_common::sqlite::execute_batch(stats_swaps::ADD_MAKER_TAKER_GUI_AND_VERSION)
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We also do migration_11 here #2046 , I guess it will show in the conflicts of this PR or the other one depending on which is merged first.
c.c. @artemii235

@shamardy shamardy requested a review from smk762 February 8, 2024 13:13
@shamardy
Copy link
Copy Markdown
Collaborator

shamardy commented Feb 8, 2024

@KomodoPlatform/qa can you please check if this fixes the issue?

Copy link
Copy Markdown

@borngraced borngraced left a comment

Choose a reason for hiding this comment

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

can this issue, #1950 be addressed in this PR if it won't take much time otherwise this LGTM! well done

@shamardy
Copy link
Copy Markdown
Collaborator

can this issue, #1950 be addressed in this PR

It's a different issue since it's related to swaps json and not stats_swaps, I think it's better to be handled in another PR.

Copy link
Copy Markdown

@smk762 smk762 left a comment

Choose a reason for hiding this comment

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

Working as expected here, thanks!

id|maker_coin|taker_coin|uuid|started_at|finished_at|maker_amount|taker_amount|is_success|maker_coin_ticker|maker_coin_platform|taker_coin_ticker|taker_coin_platform|maker_coin_usd_price|taker_coin_usd_price|maker_pubkey|taker_pubkey|maker_gui|taker_gui|maker_version|taker_version
1|MARTY|DOC|4eacacf3-3a0d-41ec-abc0-11db4edf1d0e|1708000915|1708001108|2.4|2.4|1|MARTY||DOC||||0315d9c51c657ab1be4ae9d3ab6e76a619d3bccfe830d5363fa168424c0d044732|03a93f666b9030958f282edd2904f0a33278c0c676ae132d2094840fe722f011c3|mpm|mm2_777|2.0.0-beta_b0fd99e|2.1.0-beta_9727f1f

@shamardy shamardy merged commit 0cd770a into dev Feb 15, 2024
@shamardy shamardy deleted the feat-gui-version-in-db branch February 15, 2024 18:56
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Feb 25, 2024
* dev:
  feat(zcoin): ARRR WASM implementation (GLEECBTC#1957)
  feat(trading-proto-upgrade): locked amounts, kmd burn and other impl (GLEECBTC#2046)
  fix(indexeddb): set stop on success cursor condition (GLEECBTC#2067)
  feat(config): add `max_concurrent_connections` to mm2 config (GLEECBTC#2063)
  feat(stats_swaps): add gui/mm_version in stats db (GLEECBTC#2061)
  fix(indexeddb): fix IDB cursor.continue_() call after drop (GLEECBTC#2028)
  security bump for `h2` (GLEECBTC#2062)
  fix(makerbot): allow more than one prices url in makerbot (GLEECBTC#2027)
  fix(wasm worker env): refactor direct usage of `window` (GLEECBTC#1953)
  feat(nft): nft abi in withdraw_nft RPC, clear_nft_db RPC (GLEECBTC#2039)
  refactor(utxo): refactor utxo output script creation (GLEECBTC#1960)
  feat(ETH): balance event streaming for ETH (GLEECBTC#2041)
  chore(release): bump mm2 version to 2.1.0-beta (GLEECBTC#2044)
  feat(trezor): add segwit support for withdraw with trezor (GLEECBTC#1984)
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.

4 participants