Skip to content

fix(eth-balance-events): serialize eth address using AddrToString #2440

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.

Sign up for GitHub

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

Jump to bottom
Merged
shamardy merged 1 commit into from
May 14, 2025
Merged

fix(eth-balance-events): serialize eth address using AddrToString #2440

shamardy merged 1 commit into from
May 14, 2025

Conversation

@mariocynicys
Copy link
Collaborator

@mariocynicys mariocynicys commented May 6, 2025 •
edited
Loading

as using .to_string() truncates the address and adds some weird dots inbetween that doesn't encode very well in SSE.

fixes GLEECBTC/komodo-docs-mdx#457
.

@mariocynicys
serialize eth address using AddrToString
ac243d0 
as using `.to_string()` truncates the address and adds some weird dots inbetween that doesn't encode very well in SSE
@mariocynicys mariocynicys added priority: high Important tasks that need attention soon. bug: API labels May 6, 2025
@mariocynicys
Copy link
Collaborator Author

mariocynicys commented May 6, 2025

@smk762 does this fix it?

@laruh
Copy link

laruh commented May 6, 2025 •
edited
Loading

that doesn't encode very well in SSE.

It's not actually SSE, the issue is with the Address (from ethereum types crate) type's Display implementation, which shortens the output (e.g. 0x1234…abcd).
to_string() uses Display::fmt under the hood.

Ideally, we should re review the entire eth related code and replace all address.to_string() calls with AddrToString trait method

@mariocynicys
Copy link
Collaborator Author

mariocynicys commented May 6, 2025

It's not actually SSE, the issue is with the Address (from ethereum types crate) type's Display implementation, which shortens the output (e.g. 0x1234…abcd).

didn't meant it's an SSE problem. it would have been better if it serializes with these 3 dots over SSE. but it doesn't. check the link i put in the post header.

i think this mis-serilization of that weird 3-dots character is coming from serde though. so not inherently a problem facing only SSE per se.

@laruh
Copy link

laruh commented May 6, 2025

It's not actually SSE, the issue is with the Address (from ethereum types crate) type's Display implementation, which shortens the output (e.g. 0x1234…abcd).

didn't meant it's an SSE problem. it would have been better if it serializes with these 3 dots over SSE. but it doesn't. check the link i put in the post header.

i think this mis-serilization of that weird 3-dots character is coming from serde though. so not inherently a problem facing only SSE per se.

ah, so you have more than just dots problem. just pr's description tells about "weird dots" and links just to 457 PR not to some specific comment

@mariocynicys
Copy link
Collaborator Author

mariocynicys commented May 6, 2025 •
edited
Loading

links just to 457 PR not to some specific comment

yeah. something weird with github. i double checked it's the full link (to the specific comment; in the MarkDown editor) but it still links to the PR as a whole :/

@smk762 smk762 mentioned this pull request May 7, 2025
Merged
27 tasks
@smk762 smk762 self-requested a review May 7, 2025 02:48
smk762
smk762 approved these changes May 7, 2025
View reviewed changes
Copy link

@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.

Confirm full address is now shown.

@mariocynicys mariocynicys requested a review from shamardy May 13, 2025 09:02
@shamardy shamardy merged commit 56dce19 into dev May 14, 2025
26 of 29 checks passed
@shamardy shamardy deleted the fix/balance-streaming/eth-addr-serialization branch May 14, 2025 17:55
dimxy pushed a commit that referenced this pull request May 28, 2025
Merge branch 'dev' into lr-swap-wip
d05697c 
* dev: (29 commits)
  fix(p2pk): validate expected pubkey correctly for p2pk inputs (#2408)
  chore(docs): update old urls referencing atomicdex or old docs pages (#2428)
  improvement(p2p): remove hardcoded seeds (#2439)
  fix(evm-api): find enabled erc20 token using platform ticker (#2445)
  chore(docs): add DeepWiki badge to README (#2463)
  chore(core): organize deps using workspace.dependencies (#2449)
  feat(db-arch): more dbdir to address_dir replacements (#2398)
  chore(build-artifacts): remove duplicated mm2 build artifacts (#2448)
  feat(pubkey-banning): expirable bans (#2455)
  fix(eth-balance-events): serialize eth address using AddrToString (#2440)
  chore(deps): remove base58 and replace it completely with bs58 (#2427)
  feat(tron): initial groundwork for full TRON integration (#2425)
  fix(UTXO): improve tx fee calculation and min relay fee handling (#2316)
  deps(timed-map): bump to 1.3.1 (#2413)
  improvement(tendermint): safer IBC channel handler (#2298)
  chore(release): complete v2.4.0-beta changelogs  (#2436)
  fix(event-streaming): initial addresses registration in utxo balance streaming (#2431)
  improvement(watchers): re-write use-watchers handling (#2430)
  fix(evm): make withdraw_nft work in HD mode (#2424)
  feat(taproot): support parsing taproot output address types
  ...
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Jun 8, 2025
Merge branch 'lr-swap-wip' into lr-swap-wip-2
9260f05 
* lr-swap-wip: (37 commits)
  fix custom token error name
  fix getting chain_id from protocol_data
  refactor (review): use dedicated large error cfg, add new fn to FromApiValueError, fix TODO, use experimental namespace for lr rpc, more Ticker alias
  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)
  fix(p2pk): validate expected pubkey correctly for p2pk inputs (GLEECBTC#2408)
  chore(docs): update old urls referencing atomicdex or old docs pages (GLEECBTC#2428)
  improvement(p2p): remove hardcoded seeds (GLEECBTC#2439)
  fix(evm-api): find enabled erc20 token using platform ticker (GLEECBTC#2445)
  chore(docs): add DeepWiki badge to README (GLEECBTC#2463)
  chore(core): organize deps using workspace.dependencies (GLEECBTC#2449)
  feat(db-arch): more dbdir to address_dir replacements (GLEECBTC#2398)
  chore(build-artifacts): remove duplicated mm2 build artifacts (GLEECBTC#2448)
  feat(pubkey-banning): expirable bans (GLEECBTC#2455)
  fix(eth-balance-events): serialize eth address using AddrToString (GLEECBTC#2440)
  chore(deps): remove base58 and replace it completely with bs58 (GLEECBTC#2427)
  feat(tron): initial groundwork for full TRON integration (GLEECBTC#2425)
  ...
@shamardy shamardy mentioned this pull request Jun 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shamardy shamardy shamardy approved these changes

+2 more reviewers

@smk762 smk762 smk762 approved these changes

@borngraced borngraced borngraced approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mariocynicys @laruh @smk762 @shamardy @borngraced