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

build(sdk-coin-xrp): update ripple libs #4947

Merged
merged 1 commit into from
Oct 4, 2024
Merged

build(sdk-coin-xrp): update ripple libs #4947

merged 1 commit into from
Oct 4, 2024

Conversation

hitansh-bitgo
Copy link
Contributor

Ticket: WIN-3541

@hitansh-bitgo hitansh-bitgo force-pushed the WIN-3541 branch 4 times, most recently from aab1036 to 9a11ec3 Compare October 3, 2024 04:03
@hitansh-bitgo hitansh-bitgo changed the title refactor(sdk-coin-xrp): update ripple libs build(sdk-coin-xrp): update ripple libs Oct 3, 2024
@hitansh-bitgo hitansh-bitgo marked this pull request as ready for review October 3, 2024 04:36
@hitansh-bitgo hitansh-bitgo requested review from a team as code owners October 3, 2024 04:36
@@ -91,7 +91,7 @@ describe('XRP:', function () {
txHex:
'{"TransactionType":"Payment","Account":"rBSpCz8PafXTJHppDcNnex7dYnbe3tSuFG","Destination":"rfjub8A4dpSD5nnszUFTsLprxu1W398jwc","DestinationTag":0,"Amount":"253481","Flags":2147483648,"LastLedgerSequence":1626225,"Fee":"45","Sequence":7}',
});
unsignedExplanation.id.should.equal('CB36F366F1AC25FCDB38A19F17384ED3509D9B7F063520034597852FB10A1B45');
unsignedExplanation.id.should.equal('37486621138DFB0C55FEF45FD275B565254464651A04CB02EE371F8C4A84D8CA');
Copy link
Contributor

Choose a reason for hiding this comment

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

why are the values different ?
is this a breaking change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ripple-lib 1.x only had the computeBinaryTransactionHash function that we used for both signed & unsigned txns, but xrpl 2.x has two separate functions to compute hashes for signed and unsigned transactions.
The hashSignedTx returns the same hash as computeBinaryTransactionHash since both the functions are same but hashTx(used for getting hash of unsigned Tx) uses a different HashPrefix than computeBinaryTransactionHash & hashSignedTx, hence the change in these hashes.

It most likely wouldn't break anything on WP but technically still is a breaking change so I've updated commit message

@hitansh-bitgo hitansh-bitgo force-pushed the WIN-3541 branch 2 times, most recently from a98654e to 58d17f7 Compare October 3, 2024 08:58
alebusse
alebusse previously approved these changes Oct 3, 2024
BREAKING CHANGE: explainTransaction for XRP transactions now computes
a different transaction hash than previous versions. This is because
xrpl 2.x uses different hash prefixes for computing signed and
unsigned transaction hashes

Ticket: WIN-3541
@hitansh-bitgo hitansh-bitgo merged commit 59337c2 into master Oct 4, 2024
8 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.

4 participants