Skip to content

improvement(tendermint): tendermint_tx_internal_id helper#2438

Merged
onur-ozkan merged 2 commits intodevfrom
fix-2429
Jun 9, 2025
Merged

improvement(tendermint): tendermint_tx_internal_id helper#2438
onur-ozkan merged 2 commits intodevfrom
fix-2429

Conversation

@onur-ozkan
Copy link
Copy Markdown

From #2429 (comment):

Looking at the code: https://github.com/KomodoPlatform/komodo-defi-framework/blob/3a6811af705e38380c6316e6bf41ad9aa5568fea/mm2src/coins/my_tx_history_v2.rs#L213-L222
This code is never actually invoked because the function is only used for UTXO and BCH.

Fixes #2429

…nction

Signed-off-by: onur-ozkan <work@onurozkan.dev>
borngraced
borngraced previously approved these changes May 12, 2025
tx_hash.clone()
}
TransactionType::TendermintIBCTransfer { .. } | TransactionType::CustomTendermintMsg { .. } => {
unreachable!("Tendermint never invokes this function.")
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.

i see this method is infallible. could we write a correct implementation for this if that's feasible, instead of panicking.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There is the TODO note I wrote on top of this function.

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.

it's not targeting this panic speifically tho. and who knows when this todo would be picked up.
if we can easily impl this (but only if), for my money, we better do as we lose nothing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's covering it. If you remove this function and implement it in the protocol details you won't have to do pattern matching on all the protocols.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan added the priority: high Important tasks that need attention soon. label May 26, 2025
Copy link
Copy Markdown
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks!

@onur-ozkan onur-ozkan merged commit 0070498 into dev Jun 9, 2025
19 of 25 checks passed
@onur-ozkan onur-ozkan deleted the fix-2429 branch June 9, 2025 08:30
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Jun 27, 2025
* dev: (30 commits)
  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)
  feat(tests): zcoin unit test to validate dex fee (GLEECBTC#2460)
  fix(zcoin): correctly track unconfirmed z-coin notes (GLEECBTC#2331)
  improvement(orders): remove BTC specific volume from min_trading_vol logic (GLEECBTC#2483)
  feat(ibc-routing-part-2): supporting entire Cosmos network for swaps (GLEECBTC#2476)
  fix(startup): don't initialize WalletConnect during startup (GLEECBTC#2485)
  fix(dns): better ip resolution (GLEECBTC#2487)
  improvement(event-streaming): strong type streamer IDs (GLEECBTC#2441)
  bump timed-map to `1.4.1` (GLEECBTC#2481)
  improvement(RPC): unified interface for legacy and current RPC interfaces (GLEECBTC#2450)
  improvement(tendermint): `tendermint_tx_internal_id` helper (GLEECBTC#2438)
  feat(walletconnect): add WalletConnect v2 support for EVM and Cosmos (GLEECBTC#2223)
  feat(ibc-routing-part-1): supporting entire Cosmos network for swaps (GLEECBTC#2459)
  fix(test): fix HD Wallet message signing tests (GLEECBTC#2474)
  improvement(builds): enable static CRT linking for MSVC builds (GLEECBTC#2464)
  feat(wallet): implement HD multi-address support for message signing (GLEECBTC#2432)
  ...

# Conflicts:
#	mm2src/coins/qrc20.rs
#	mm2src/mm2_main/src/lp_swap/maker_swap.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement: API priority: high Important tasks that need attention soon. status: pending review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants