Skip to content

Fix transaction metadata loading issues#5449

Merged
samholmes merged 2 commits intodevelopfrom
sam/tx-metadata-shmetadata
Feb 11, 2025
Merged

Fix transaction metadata loading issues#5449
samholmes merged 2 commits intodevelopfrom
sam/tx-metadata-shmetadata

Conversation

@samholmes
Copy link
Contributor

@samholmes samholmes commented Feb 11, 2025

  • Fix yarn.lock
  • Rename txs identifier
  • Fix bug with relevant changedTxs set

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

@samholmes samholmes force-pushed the sam/tx-metadata-shmetadata branch from feb4129 to a0e57be Compare February 11, 2025 00:22
Copy link
Contributor

@swansontec swansontec left a comment

Choose a reason for hiding this comment

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

This PR is perfect.

let relevant = false
for (const tx of txs) {
if (tx.tokenId === tokenId) {
if (tx.tokenId === tokenId && streamedTxs.some(streamedTx => streamedTx.txid === tx.txid)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is potentially O(n^2), but only if the core / engines have bugs that spam onTransactionChanged events. However, spamming needless events is already inefficient, so let's leave this as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it

@swansontec
Copy link
Contributor

swansontec commented Feb 11, 2025

Actually, the first commit is probably not needed, because these two fields randomly flip-flop back & forth, and we've never figured out what was the difference between the different dev's machines that was causing it. So just drop the commit, and the next time you yarn add something, just let them change then.

@samholmes samholmes force-pushed the sam/tx-metadata-shmetadata branch from a0e57be to c7466a1 Compare February 11, 2025 02:11
Only transactions that have been streamed are relevant for the
changedTxs set because we _always_ stomp the changed tx onto the
streamedTxs array.
@samholmes samholmes force-pushed the sam/tx-metadata-shmetadata branch from f2590b8 to 3472d7c Compare February 11, 2025 02:17
@samholmes samholmes enabled auto-merge February 11, 2025 02:17
@samholmes samholmes merged commit 4c12abe into develop Feb 11, 2025
2 checks passed
@samholmes samholmes deleted the sam/tx-metadata-shmetadata branch February 11, 2025 02:28
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.

2 participants