Skip to content
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

fix(nft): add token_id field to the tx history primary key, fix balance #2209

Merged
merged 23 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
51eb543
update multi_index in wasm NftTransferHistoryTable and PRIMARY KEY in…
laruh Sep 3, 2024
2bdee4f
fix erc1155_balance, check that "balanceOf" ERC11155 returns the exac…
laruh Sep 3, 2024
416b23c
Merge remote-tracking branch 'origin/dev' into fix-nft-tx-history-pri…
laruh Sep 12, 2024
a493f3f
require amount: Option<BigUint> in WithdrawErc1155
laruh Sep 12, 2024
48a7f5b
cover sql nft tx history migration
laruh Sep 12, 2024
1da9cb8
Merge remote-tracking branch 'origin/dev' into fix-nft-tx-history-pri…
laruh Sep 13, 2024
5cae2d4
make migration function name clearer
laruh Sep 13, 2024
9cebf62
wasm migration wip
laruh Sep 15, 2024
df164d4
copy_store_data_sync for NftTransferHistoryTable migration
laruh Sep 15, 2024
041f99c
change version check and add logs
laruh Sep 15, 2024
e8be556
use "else if old_version == 1 && new_version == 2" check
laruh Sep 16, 2024
c42e941
Merge remote-tracking branch 'origin/dev' into fix-nft-tx-history-pri…
laruh Sep 30, 2024
12bdb27
WIP: update on_upgrade_needed for wasm nft tables
laruh Oct 10, 2024
7511d94
Merge remote-tracking branch 'origin/dev' into fix-nft-tx-history-pri…
laruh Oct 11, 2024
14bef0d
update old/new versions handle, remove unused code
laruh Oct 11, 2024
a3ba820
avoid duplication when we crate tx history sql str, migrate_tx_histor…
laruh Oct 13, 2024
5c14c09
update comment in migrate_tx_history_table_to_schema_v2
laruh Oct 13, 2024
38ac974
cover schema_table deletion in RPC clear_nft_db
laruh Oct 13, 2024
43d7ba2
fix sql migration bug
laruh Oct 14, 2024
a5486e1
Merge remote-tracking branch 'origin/dev' into fix-nft-tx-history-pri…
laruh Oct 26, 2024
ef3dd1e
review: replace BigDecimal by BigUint in NFT withdraw
laruh Oct 26, 2024
7636464
review: use progressive upgrade pattern for IDB and sql schemas
laruh Oct 27, 2024
f44bc38
review: remove break to allow straightforward version-by-version upg…
laruh Nov 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion mm2src/coins/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4434,7 +4434,8 @@ impl EthCoin {
))
},
};
let wallet_amount = u256_to_big_decimal(wallet_amount_uint, self.decimals)?;
// The "balanceOf" function in ERC1155 standard returns the exact count of tokens held by address without any decimals or scaling factors
let wallet_amount = wallet_amount_uint.to_string().parse::<BigDecimal>()?;
shamardy marked this conversation as resolved.
Show resolved Hide resolved
Ok(wallet_amount)
}

Expand Down
2 changes: 1 addition & 1 deletion mm2src/coins/lp_coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2727,7 +2727,7 @@ pub enum BalanceError {
UnexpectedDerivationMethod(UnexpectedDerivationMethod),
#[display(fmt = "Wallet storage error: {}", _0)]
WalletStorageError(String),
#[from_stringify("Bip32Error", "NumConversError")]
#[from_stringify("Bip32Error", "NumConversError", "ParseBigDecimalError")]
#[display(fmt = "Internal: {}", _0)]
Internal(String),
}
Expand Down
7 changes: 6 additions & 1 deletion mm2src/coins/nft/nft_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,12 @@ cross_test!(test_add_get_transfers, {
.clone();
assert_eq!(transfer1.block_number, 28056721);
let transfer2 = storage
.get_transfer_by_tx_hash_and_log_index(&chain, TX_HASH.to_string(), LOG_INDEX)
.get_transfer_by_tx_hash_log_index_token_id(
&chain,
TX_HASH.to_string(),
LOG_INDEX,
BigUint::from_str("214300047253").unwrap(),
)
.await
.unwrap()
.unwrap();
Expand Down
8 changes: 4 additions & 4 deletions mm2src/coins/nft/storage/db_test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ pub(crate) fn nft_transfer_history() -> Vec<NftTransferHistory> {
transaction_index: Some(198),
log_index: 495,
value: Default::default(),
transaction_type: Some("Single".to_string()),
transaction_type: Some("Batch".to_string()),
token_address: Address::from_str("0xfd913a305d70a60aac4faac70c739563738e1f81").unwrap(),
from_address: Address::from_str("0x6fad0ec6bb76914b2a2a800686acc22970645820").unwrap(),
to_address: Address::from_str("0xf622a6c52c94b500542e2ae6bcad24c53bc5b6a2").unwrap(),
Expand All @@ -284,15 +284,15 @@ pub(crate) fn nft_transfer_history() -> Vec<NftTransferHistory> {
confirmations: 0,
};

// Same as transfer1 but with different log_index, meaning that transfer1 and transfer2 are part of one batch/multi token transaction
// Same as transfer1 (identical tx hash and log index) but with different token_id, meaning that transfer1 and transfer2 are part of one batch/multi token transaction
let transfer2 = NftTransferHistory {
common: NftTransferCommon {
block_hash: Some("0x3d68b78391fb3cf8570df27036214f7e9a5a6a45d309197936f51d826041bfe7".to_string()),
transaction_hash: "0x1e9f04e9b571b283bde02c98c2a97da39b2bb665b57c1f2b0b733f9b681debbe".to_string(),
transaction_index: Some(198),
log_index: 496,
log_index: 495,
value: Default::default(),
transaction_type: Some("Single".to_string()),
transaction_type: Some("Batch".to_string()),
token_address: Address::from_str("0xfd913a305d70a60aac4faac70c739563738e1f81").unwrap(),
from_address: Address::from_str("0x6fad0ec6bb76914b2a2a800686acc22970645820").unwrap(),
to_address: Address::from_str("0xf622a6c52c94b500542e2ae6bcad24c53bc5b6a2").unwrap(),
Expand Down
3 changes: 2 additions & 1 deletion mm2src/coins/nft/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,12 @@ pub trait NftTransferHistoryStorageOps {
token_id: BigUint,
) -> MmResult<Vec<NftTransferHistory>, Self::Error>;

async fn get_transfer_by_tx_hash_and_log_index(
async fn get_transfer_by_tx_hash_log_index_token_id(
&self,
chain: &Chain,
transaction_hash: String,
log_index: u32,
token_id: BigUint,
) -> MmResult<Option<NftTransferHistory>, Self::Error>;

/// Updates the metadata for NFT transfers identified by their token address and ID.
Expand Down
9 changes: 5 additions & 4 deletions mm2src/coins/nft/storage/sql_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn create_transfer_history_table_sql(chain: &Chain) -> Result<String, SqlError>
image_domain TEXT,
token_name TEXT,
details_json TEXT,
PRIMARY KEY (transaction_hash, log_index)
PRIMARY KEY (transaction_hash, log_index, token_id)
shamardy marked this conversation as resolved.
Show resolved Hide resolved
);",
safe_table_name.inner()
);
Expand Down Expand Up @@ -1121,22 +1121,23 @@ impl NftTransferHistoryStorageOps for AsyncMutexGuard<'_, AsyncConnection> {
.map_to_mm(AsyncConnError::from)
}

async fn get_transfer_by_tx_hash_and_log_index(
async fn get_transfer_by_tx_hash_log_index_token_id(
&self,
chain: &Chain,
transaction_hash: String,
log_index: u32,
token_id: BigUint,
) -> MmResult<Option<NftTransferHistory>, Self::Error> {
let table_name = chain.transfer_history_table_name()?;
let sql = format!(
"SELECT * FROM {} WHERE transaction_hash=?1 AND log_index = ?2",
"SELECT * FROM {} WHERE transaction_hash=?1 AND log_index = ?2 AND token_id = ?3",
table_name.inner()
);
self.call(move |conn| {
let transfer = query_single_row(
conn,
&sql,
[transaction_hash, log_index.to_string()],
[transaction_hash, log_index.to_string(), token_id.to_string()],
transfer_history_from_row,
)?;
Ok(transfer)
Expand Down
29 changes: 17 additions & 12 deletions mm2src/coins/nft/storage/wasm/wasm_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,18 +547,20 @@ impl NftTransferHistoryStorageOps for NftCacheIDBLocked<'_> {
.collect()
}

async fn get_transfer_by_tx_hash_and_log_index(
async fn get_transfer_by_tx_hash_log_index_token_id(
&self,
chain: &Chain,
transaction_hash: String,
log_index: u32,
token_id: BigUint,
) -> MmResult<Option<NftTransferHistory>, Self::Error> {
let db_transaction = self.get_inner().transaction().await?;
let table = db_transaction.table::<NftTransferHistoryTable>().await?;
let index_keys = MultiIndex::new(NftTransferHistoryTable::CHAIN_TX_HASH_LOG_INDEX_INDEX)
let index_keys = MultiIndex::new(NftTransferHistoryTable::CHAIN_TX_HASH_LOG_INDEX_TOKEN_ID_INDEX)
.with_value(chain.to_string())?
.with_value(&transaction_hash)?
.with_value(log_index)?;
.with_value(log_index)?
.with_value(BeBigUint::from(token_id))?;

if let Some((_item_id, item)) = table.get_item_by_unique_multi_index(index_keys).await? {
Ok(Some(transfer_details_from_item(item)?))
Expand Down Expand Up @@ -602,10 +604,11 @@ impl NftTransferHistoryStorageOps for NftCacheIDBLocked<'_> {
}
drop_mutability!(transfer);

let index_keys = MultiIndex::new(NftTransferHistoryTable::CHAIN_TX_HASH_LOG_INDEX_INDEX)
let index_keys = MultiIndex::new(NftTransferHistoryTable::CHAIN_TX_HASH_LOG_INDEX_TOKEN_ID_INDEX)
.with_value(&chain_str)?
.with_value(&transfer.common.transaction_hash)?
.with_value(transfer.common.log_index)?;
.with_value(transfer.common.log_index)?
.with_value(BeBigUint::from(transfer.token_id.clone()))?;

let item = NftTransferHistoryTable::from_transfer_history(&transfer)?;
table.replace_item_by_unique_multi_index(index_keys, &item).await?;
Expand Down Expand Up @@ -691,10 +694,11 @@ impl NftTransferHistoryStorageOps for NftCacheIDBLocked<'_> {
transfer.common.possible_spam = possible_spam;
drop_mutability!(transfer);

let index_keys = MultiIndex::new(NftTransferHistoryTable::CHAIN_TX_HASH_LOG_INDEX_INDEX)
let index_keys = MultiIndex::new(NftTransferHistoryTable::CHAIN_TX_HASH_LOG_INDEX_TOKEN_ID_INDEX)
.with_value(&chain_str)?
.with_value(&transfer.common.transaction_hash)?
.with_value(transfer.common.log_index)?;
.with_value(transfer.common.log_index)?
.with_value(BeBigUint::from(transfer.token_id.clone()))?;

let item = NftTransferHistoryTable::from_transfer_history(&transfer)?;
table.replace_item_by_unique_multi_index(index_keys, &item).await?;
Expand Down Expand Up @@ -777,10 +781,11 @@ async fn update_transfer_phishing_for_index(
transfer.possible_phishing = possible_phishing;
drop_mutability!(transfer);
let transfer_item = NftTransferHistoryTable::from_transfer_history(&transfer)?;
let index_keys = MultiIndex::new(NftTransferHistoryTable::CHAIN_TX_HASH_LOG_INDEX_INDEX)
let index_keys = MultiIndex::new(NftTransferHistoryTable::CHAIN_TX_HASH_LOG_INDEX_TOKEN_ID_INDEX)
.with_value(chain)?
.with_value(&transfer.common.transaction_hash)?
.with_value(transfer.common.log_index)?;
.with_value(transfer.common.log_index)?
.with_value(BeBigUint::from(transfer.token_id))?;
table
.replace_item_by_unique_multi_index(index_keys, &transfer_item)
.await?;
Expand Down Expand Up @@ -951,7 +956,7 @@ pub(crate) struct NftTransferHistoryTable {
}

impl NftTransferHistoryTable {
const CHAIN_TX_HASH_LOG_INDEX_INDEX: &str = "chain_tx_hash_log_index_index";
const CHAIN_TX_HASH_LOG_INDEX_TOKEN_ID_INDEX: &str = "chain_tx_hash_log_index_token_idindex";

fn from_transfer_history(transfer: &NftTransferHistory) -> WasmNftCacheResult<NftTransferHistoryTable> {
let details_json =
Expand Down Expand Up @@ -992,8 +997,8 @@ impl TableSignature for NftTransferHistoryTable {
false,
)?;
table.create_multi_index(
Self::CHAIN_TX_HASH_LOG_INDEX_INDEX,
&["chain", "transaction_hash", "log_index"],
Self::CHAIN_TX_HASH_LOG_INDEX_TOKEN_ID_INDEX,
&["chain", "transaction_hash", "log_index", "token_id"],
true,
)?;
table.create_multi_index(CHAIN_BLOCK_NUMBER_INDEX, &["chain", "block_number"], false)?;
Expand Down
15 changes: 10 additions & 5 deletions mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ fn geth_erc712_owner(token_id: U256) -> Address {
block_on(erc721_contract.query("ownerOf", Token::Uint(token_id), None, Options::default(), None)).unwrap()
}

fn mint_erc1155(to_addr: Address, token_id: U256, amount: U256) {
fn mint_erc1155(to_addr: Address, token_id: U256, amount: u32) {
let _guard = GETH_NONCE_LOCK.lock().unwrap();
let erc1155_contract =
Contract::from_json(GETH_WEB3.eth(), geth_erc1155_contract(), ERC1155_TEST_ABI.as_bytes()).unwrap();
Expand All @@ -182,7 +182,7 @@ fn mint_erc1155(to_addr: Address, token_id: U256, amount: U256) {
(
Token::Address(to_addr),
Token::Uint(token_id),
Token::Uint(amount),
Token::Uint(U256::from(amount)),
Token::Bytes("".into()),
),
geth_account(),
Expand All @@ -201,10 +201,15 @@ fn mint_erc1155(to_addr: Address, token_id: U256, amount: U256) {
))
.unwrap();

// check that "balanceOf" from ERC11155 returns the exact amount of token without any decimals or scaling factors
let balance_dec = balance.to_string().parse::<BigDecimal>().unwrap();
assert_eq!(
balance, amount,
balance_dec,
BigDecimal::from(amount),
"The balance of tokenId {:?} for address {:?} does not match the expected amount {:?}.",
token_id, to_addr, amount
token_id,
to_addr,
amount
);
}

Expand Down Expand Up @@ -382,7 +387,7 @@ fn global_nft_with_random_privkey(
if let Some(nft_type) = nft_type {
match nft_type {
TestNftType::Erc1155 { token_id, amount } => {
mint_erc1155(my_address, U256::from(token_id), U256::from(amount));
mint_erc1155(my_address, U256::from(token_id), amount);
block_on(fill_erc1155_info(
&global_nft,
geth_erc1155_contract(),
Expand Down
Loading