-
Notifications
You must be signed in to change notification settings - Fork 117
fix(tx-history): token information and query #2404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
21cd458
cee056d
4b47807
0be59b1
5b8e290
3fc5e0e
25bb8ac
b6eaeca
9c31002
3aea946
04392d2
2812ff8
528291d
dd550dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| use crate::hd_wallet::{AddressDerivingError, DisplayAddress, InvalidBip44ChainError}; | ||
| use crate::tendermint::{TENDERMINT_ASSET_PROTOCOL_TYPE, TENDERMINT_COIN_PROTOCOL_TYPE}; | ||
| use crate::tendermint::{BCH_COIN_PROTOCOL_TYPE, BCH_TOKEN_PROTOCOL_TYPE, TENDERMINT_ASSET_PROTOCOL_TYPE, | ||
| TENDERMINT_COIN_PROTOCOL_TYPE}; | ||
| use crate::tx_history_storage::{CreateTxHistoryStorageError, FilteringAddresses, GetTxHistoryFilters, | ||
| TxHistoryStorageBuilder, WalletId}; | ||
| use crate::utxo::utxo_common::big_decimal_from_sat_unsigned; | ||
|
|
@@ -209,7 +210,8 @@ impl<'a, Addr: Clone + DisplayAddress + Eq + std::hash::Hash, Tx: Transaction> T | |
| bytes_for_hash.extend_from_slice(&token_id.0); | ||
| sha256(&bytes_for_hash).to_vec().into() | ||
| }, | ||
| TransactionType::CustomTendermintMsg { token_id, .. } => { | ||
| TransactionType::TendermintIBCTransfer { token_id } | ||
| | TransactionType::CustomTendermintMsg { token_id, .. } => { | ||
| if let Some(token_id) = token_id { | ||
| let mut bytes_for_hash = tx_hash.0.clone(); | ||
| bytes_for_hash.extend_from_slice(&token_id.0); | ||
|
Comment on lines
+213
to
217
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where as here in tx_hist, we do embed token_id into the internal identifer
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That looks weird. 🤔 We shouldn't do that probably, but I don't want remove it and go that far in this PR... |
||
|
|
@@ -223,8 +225,7 @@ impl<'a, Addr: Clone + DisplayAddress + Eq + std::hash::Hash, Tx: Transaction> T | |
| | TransactionType::ClaimDelegationRewards | ||
| | TransactionType::FeeForTokenTx | ||
| | TransactionType::StandardTransfer | ||
| | TransactionType::NftTransfer | ||
| | TransactionType::TendermintIBCTransfer => tx_hash.clone(), | ||
| | TransactionType::NftTransfer => tx_hash.clone(), | ||
| }; | ||
|
|
||
| TransactionDetails { | ||
|
|
@@ -414,11 +415,6 @@ where | |
| .transactions | ||
| .into_iter() | ||
| .map(|mut details| { | ||
| // it can be the platform ticker instead of the token ticker for a pre-saved record | ||
| if details.coin != request.coin { | ||
| details.coin = request.coin.clone(); | ||
| } | ||
|
|
||
| // TODO | ||
| // !! temporary solution !! | ||
| // for tendermint, tx_history_v2 implementation doesn't include amount parsing logic. | ||
|
|
@@ -468,6 +464,16 @@ where | |
| }, | ||
| } | ||
| }, | ||
| BCH_COIN_PROTOCOL_TYPE | BCH_TOKEN_PROTOCOL_TYPE => { | ||
| // SLP tokens are part of BCH transactions and SLP transactions might be stored with the BCH ticker. | ||
| // Ideally, we should avoid this workaround and instead fix the incorrect ticker logic when inserting | ||
| // transactions with the wrong ticker. | ||
| // | ||
| // Original PR: https://github.com/KomodoPlatform/komodo-defi-framework/pull/1175. | ||
| if details.coin != request.coin { | ||
| details.coin = request.coin.clone(); | ||
| } | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea how this helps to other protocols but it's buggy (hiding actual token tickers) on Tendermint, so I moved it here.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparently this was added for BCH and SLP tx history, SLP tokens are included in a BCH transaction and it might be saved with a BCH ticker. EVMs were not added to v2 tx history so the only tokens protocol that uses this is slp, maybe we should specify this.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. awesome let's please add a comment about this then.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a TODO note.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would actually use the BCH/SLP protocol instead of
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done here 2812ff8 can one of you @mariocynicys @onur-ozkan please check it and resolve this commit if it's fine. I will then merge the PR. |
||
| }, | ||
| _ => {}, | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ use keys::KeyPair; | |
| use mm2_core::mm_ctx::MmArc; | ||
| use mm2_err_handle::prelude::*; | ||
| use mm2_number::MmNumber; | ||
| use primitives::hash::H256; | ||
| use rpc::v1::types::Bytes as BytesJson; | ||
| use serde_json::Value as Json; | ||
| use std::ops::Deref; | ||
|
|
@@ -95,6 +96,11 @@ impl TendermintToken { | |
| }; | ||
| Ok(TendermintToken(Arc::new(token_impl))) | ||
| } | ||
|
|
||
| fn token_id(&self) -> BytesJson { | ||
| let denom_hash = sha256(self.denom.as_ref().to_lowercase().as_bytes()); | ||
| H256::from(denom_hash.take()).to_vec().into() | ||
| } | ||
| } | ||
|
|
||
| #[async_trait] | ||
|
|
@@ -513,9 +519,11 @@ impl MmCoin for TendermintToken { | |
| internal_id, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in withdraw, the internal ID for the token case doesn't seem to consider the token_id https://github.com/KomodoPlatform/komodo-defi-framework/blob/971b40529d076969ad1b4391594876b463de6510/mm2src/coins/tendermint/tendermint_token.rs#L497-L500 ... |
||
| kmd_rewards: None, | ||
| transaction_type: if is_ibc_transfer { | ||
| TransactionType::TendermintIBCTransfer | ||
| TransactionType::TendermintIBCTransfer { | ||
| token_id: Some(token.token_id()), | ||
| } | ||
| } else { | ||
| TransactionType::StandardTransfer | ||
| TransactionType::TokenTransfer(token.token_id()) | ||
| }, | ||
| memo: Some(memo), | ||
| }) | ||
|
|
@@ -542,7 +550,7 @@ impl MmCoin for TendermintToken { | |
| Box::new(futures01::future::err(())) | ||
| } | ||
|
|
||
| fn history_sync_status(&self) -> HistorySyncState { HistorySyncState::NotEnabled } | ||
| fn history_sync_status(&self) -> HistorySyncState { self.platform_coin.history_sync_status() } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. quoting from the PR's post:
Q: is this the change that made it possible to query tendermint token history? if not, could you guide me to the change please, as there is no dedicated commit for it.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, not this. It's the changes made in |
||
|
|
||
| fn get_trade_fee(&self) -> Box<dyn Future<Item = TradeFee, Error = String> + Send> { | ||
| Box::new(futures01::future::err("Not implemented".into())) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.