Skip to content

Remove custom Buffer.subarray polyfill#795

Merged
Shaptic merged 3 commits intostellar:masterfrom
lightsail-network:remove-buffer-patch
Jun 20, 2025
Merged

Remove custom Buffer.subarray polyfill#795
Shaptic merged 3 commits intostellar:masterfrom
lightsail-network:remove-buffer-patch

Conversation

@overcat
Copy link
Contributor

@overcat overcat commented Mar 27, 2025

In the latest version of React Native, the current patch implementation seems to not work smoothly, so let's remove it and instead use @exodus/patch-broken-hermes-typed-arrays to fix this issue. The @exodus/patch-broken-hermes-typed-arrays is used by well-known applications like Rabby Wallet and Exodus, so I believe it should be trustworthy.

P.S. Without removing this patch, using @exodus/patch-broken-hermes-typed-arrays can also make everything work smoothly, so there is no rush to release a new version.

stellar/js-xdr#128

Copy link
Contributor

@Shaptic Shaptic left a comment

Choose a reason for hiding this comment

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

Nice, a robust package you can just install is way better than a package-specific monkeypatch. Since this is breaking I'll have to merge it as part of Protocol 23 but I'll approve it. I'll also have to remember to add this caveat to the RN instructions.

@overcat
Copy link
Contributor Author

overcat commented Apr 29, 2025

Hi @Shaptic, I have recently been working on Ledger Live and Trezor Suite, and I feel that asking them to globally introduce patch-broken-hermes-typed-arrays is not particularly realistic. The Ledger Live team and I tried a global introduction, but it broke the Polkadot integration (I haven't investigated the reason yet; it's quite chaotic here). So I'm wondering if we should keep the Buffer Patch in the SDK.

Additionally, this Buffer patch in the SDK is not particularly complete and does not cover all scenarios. I wonder if we can directly introduce patch-broken-hermes-typed-arrays into this SDK to fix this issue?

Update: Let me see what kind of feedback the Trezor team will provide.

@overcat
Copy link
Contributor Author

overcat commented Apr 29, 2025

Hi @Shaptic, I'm a bit lost. If we remove this polyfill and patch it directly where it's used, like in Trezor Suite, will it affect the Stellar SDK internally? It seems not? Is this related to our webpack configuration?

Update: They do not use the same Buffer.

@overcat
Copy link
Contributor Author

overcat commented Apr 29, 2025

I created a repository to reproduce the current SDK's strange behavior. https://github.com/overcat/StellarSDKBufferDebugApp/blob/main/app/(tabs)/index.tsx#L7-L60

If you want to run it, you need to configure the environment according to RN's instructions: https://reactnative.dev/docs/set-up-your-environment

This is the output on my end:

 (NOBRIDGE) LOG  Memo XDR: AAAAAQAAAAtIZWxsbyB3b3JsZAA=
 (NOBRIDGE) LOG  ----------------------------------------------------
 (NOBRIDGE) LOG  Transaction XDR: AAAAAgAAAAA0M9clt3S3+wc6seyCYDyVic6vmnjjhZjFZ96tcMQFVwAAAGQAAAAAAAAAAgAAAAEAAAAAAAAAAAAAAABoEMDNAAAAAQAAAAtIZWxsbyB3b3JsZAAAAAABAAAAAAAAAAEAAAAACz8P0xh5CP5PLpwPKq/AqrJVsDeqstmO9euSCApoB94AAAABREVNTwAAAADhCnWmdZbYIpNgKp76E8nE2eOlj0Y3PYWRiGNiCY68eQAAAAJUHqiwAAAAAAAAAAA=
 (NOBRIDGE) LOG  ----------------------------------------------------
 (NOBRIDGE) LOG  tx2 parsing failed
 (NOBRIDGE) LOG  [Error: Asset code is invalid (maximum alphanumeric, 12 characters at max)]
 (NOBRIDGE) LOG  tx3 parsed successfully

@overcat
Copy link
Contributor Author

overcat commented Apr 29, 2025

It seems that polyfill in js-xdr is effective, but not in stellar-base. Maybe it's a configuration issue? Exploring...

@overcat
Copy link
Contributor Author

overcat commented Apr 30, 2025

🤦‍♂️ The stellar-sdk does not use the compiled stellar-base, so the Buffer used by stellar-base is the unpatched version provided by stellar-sdk, which leads to this strange behavior. Why doesn't stellar-sdk use the compiled stellar-base? I don't know.

#647 (comment)

Patching Buffer in the stellar-sdk package can fix this issue, but it's still quite strange. It would be great if Facebook could fix the bug in the engine 😭

@overcat
Copy link
Contributor Author

overcat commented May 1, 2025

#798 should be able to solve the above problem

Copy link
Contributor

@Shaptic Shaptic left a comment

Choose a reason for hiding this comment

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

@overcat do we still want this in? It's hard to say based on your later comments in the PR.

@overcat
Copy link
Contributor Author

overcat commented Jun 19, 2025

Hi @Shaptic, I think we can merge it.

@Shaptic Shaptic merged commit 01da49a into stellar:master Jun 20, 2025
9 checks passed
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