Applied fixes from master#922
Conversation
quietbits
commented
Mar 9, 2026
- Some fixes #891
- Some fixes 2 #892
|
Size Change: +8.99 kB (+0.25%) Total Size: 3.61 MB
|
There was a problem hiding this comment.
Pull request overview
This PR backports a set of fixes from master (per #891 and #892) into this branch, covering Soroban token parsing, authorization nonce generation, integer sizing, transaction immutability, and TransactionBuilder cloning behavior.
Changes:
- Fixes/guards around numeric parsing & sizing (Soroban token amounts,
ScInt,Memo.id). - Makes
TransactionBase.networkPassphraseimmutable and updates cloning/timebounds/extra signers behaviors inTransactionBuilder. - Adds/updates unit tests for the above and corrects a few typos in messages/docs.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| type_validation/transaction_base.d.ts | Aligns d.ts with TransactionBase networkPassphrase immutability change. |
| type_validation/signerkey.d.ts | Fixes spelling in doc comment (“primarly” → “primarily”). |
| test/unit/transaction_test.js | Adds coverage for networkPassphrase immutability; fixes error message typo expectation. |
| test/unit/transaction_builder_test.js | Adds coverage for floored Date timebounds + cloneFrom fee/extraSigners behavior; reorganizes tests. |
| test/unit/soroban.test.ts | Adds test for “too many decimal places” in parseTokenAmount. |
| test/unit/numbers/sc_int.test.ts | Extends coverage for string inputs and boundary sizing to u128/u256. |
| test/unit/memo.test.ts | Updates expectations to uint64 + adds negative/decimal/overflow coverage. |
| test/unit/fee_bump_transation_test.js | Fixes error message typo expectation. |
| test/unit/auth_test.js | Adds regression tests ensuring nonce generation consumes all 8 bytes. |
| src/transaction_builder.js | Floors Date timebounds seconds, fixes cloneFrom fee division, and clones extraSigners as StrKeys; docstring fixes. |
| src/transaction_base.ts | Makes networkPassphrase setter throw (immutable) and fixes error message typo. |
| src/transaction.js | Updates extraSigners JSDoc to reflect XDR signer keys. |
| src/soroban.ts | Adds guard for too many decimal places in parseTokenAmount. |
| src/signerkey.ts | Fixes signedPayload decode to respect the embedded length; doc typo fix. |
| src/numbers/sc_int.ts | Normalizes parsing via BigInt(value) and improves sizing logic by using bigint consistently. |
| src/memo.ts | Strengthens Memo.id validation to enforce uint64 constraints. |
| src/keypair.ts | Makes verify() return false on thrown verification errors. |
| src/auth.js | Fixes nonce generation to correctly build full int64 from 8 bytes (avoids Int32 truncation). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| describe("addSacTransferOperation", function () { | ||
| const { xdr } = StellarBase; | ||
| const networkPassphrase = StellarBase.Networks.TESTNET; | ||
|
|
||
| const SOURCE_ACCOUNT = | ||
| "GCEZWKCA5VLDNRLN3RPRJMRZOX3Z6G5CHCGSNFHEYVXM3XOJMDS674JZ"; | ||
| const DESTINATION_ACCOUNT = | ||
| "GDJJRRMBK4IWLEPJGIE6SXD2LP7REGZODU7WDC3I2D6MR37F4XSHBKX2"; |
There was a problem hiding this comment.
Why did these tests get removed?
There was a problem hiding this comment.
These tests are still there, starting at line 229. Maybe we had duplicate blocks? This file reflects exactly what we have in master now.