feat(abstract-eth): support txn bulding for v4 wallet on eth chain #4423
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -94,12 +94,12 @@ export const SEND_TERC_DATA = | |||
| export const CONTRACT_TOKEN_CUSD_ADDRESS = '0xa561131a1C8aC25925FB848bCa45A74aF61e5A38'; | |||
|
|
|||
| export const SEND_TX_BROADCAST_LEGACY = | |||
| '0xf901ca02843b9aca0083b8a1a0948f977e912ef500548a0c3be6ddde9899f1199b8180b901643912521500000000000000000000000019645032c7f1533395d44a629462e751084d3e4c000000000000000000000000000000000000000000000000000000003b9aca0000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000005ec67e28000000000000000000000000000000000000000000000000000000000000000500000000000000000000000000000000000000000000000000000000000000e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000417e522e602252e010f97b81b1a83128cf283dcef2ea06c221a236c1c11883adfc222a46f4c9a6ab8cd1d5dd1e963ffa443de88c9cebf755c399702dc00b0b55d61b0000000000000000000000000000000000000000000000000000000000000078a065751b9ff267e551cdc3e304e4314ac395be24b9071b10d90067215414782cb5a0111e39c47d856150fdcf521417aa3c1eea64861f349009be969c835e2bbe0425'; | |||
| '0xf901cc02843b9aca0083b8a1a0948f977e912ef500548a0c3be6ddde9899f1199b8180b901643912521500000000000000000000000019645032c7f1533395d44a629462e751084d3e4c000000000000000000000000000000000000000000000000000000003b9aca0000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000005ec67e28000000000000000000000000000000000000000000000000000000000000000500000000000000000000000000000000000000000000000000000000000000e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000417e522e602252e010f97b81b1a83128cf283dcef2ea06c221a236c1c11883adfc222a46f4c9a6ab8cd1d5dd1e963ffa443de88c9cebf755c399702dc00b0b55d61b000000000000000000000000000000000000000000000000000000000000008284f4a0375b0731252b8a4a6f54cd3ee8078e5ba8a89168c8b9b3a49a7a3fb60984b49fa029de0bb07d891c5ad4273016055e474b0c70fa288008eadfaa16ec73b6b78427'; | |||
There was a problem hiding this comment.
updated the txHex to use hteth
| @@ -95,7 +101,7 @@ export class TransferBuilder { | |||
| throw new InvalidParameterValueError('Invalid expiration time'); | |||
| } | |||
|
|
|||
| signAndBuild(chainId?: string, coinUsesNonPackedEncodingForTxData?: boolean): string { | |||
| signAndBuild(chainId: string, coinUsesNonPackedEncodingForTxData?: boolean): string { | |||
There was a problem hiding this comment.
made chainId mandatory
There was a problem hiding this comment.
wondering if this should be considered as a breaking change?
There was a problem hiding this comment.
don't we always pass chainId here
There was a problem hiding this comment.
Actually, there we always pass chainId, so it won't affect wallet platform and WRW most likely. But if some of our user is directly using signAndBuild method, so I was thinking it as a breaking change from that perspective
There was a problem hiding this comment.
If the API changes and is exposed publicly, it is a breaking change. Please mark the commit as such.
| txBuilder.sign({ key: testData.PRIVATE_KEY }); | ||
| await txBuilder.build(); | ||
| const operationData = txBuilder.transfer().getOperationData(); | ||
| should.equal(operationData[1][0], '17000'); |
There was a problem hiding this comment.
this tests ensure the prefix is chainId for walletVersion 4
OttoAllmendinger
previously approved these changes
Apr 11, 2024
zahin-mohammad
previously requested changes
Apr 11, 2024
mullapudipruthvik
force-pushed
the
WIN-2755-support-v4-wallet-txn-building-eth
branch
from
fc2f454
to
8786ff3
Compare
mullapudipruthvik
force-pushed
the
WIN-2755-support-v4-wallet-txn-building-eth
branch
from
8786ff3
to
db2210a
Compare
gianchandania
previously approved these changes
Apr 15, 2024
dpkjnr
previously approved these changes
Apr 15, 2024
mullapudipruthvik
dismissed
zahin-mohammad’s stale review
addressed Zahin's comment
the-smooth-operator
previously approved these changes
Apr 22, 2024
Ticket: WIN:2755 TICKET: WIN-2755 BREAKING CHANGE: Made ChainID mandatory for signAndBuild method
mullapudipruthvik
dismissed stale reviews from the-smooth-operator, dpkjnr, and gianchandania
via
d8aabea
mullapudipruthvik
force-pushed
the
WIN-2755-support-v4-wallet-txn-building-eth
branch
from
db2210a
to
d8aabea
Compare
mullapudipruthvik
requested review from
OttoAllmendinger,
dpkjnr,
the-smooth-operator and
gianchandania
|
rebased due to merge conflict |
sumanmukherjee03
approved these changes
Apr 22, 2024
zahin-mohammad
approved these changes
Apr 22, 2024
| @@ -95,7 +101,7 @@ export class TransferBuilder { | |||
| throw new InvalidParameterValueError('Invalid expiration time'); | |||
| } | |||
|
|
|||
| signAndBuild(chainId?: string, coinUsesNonPackedEncodingForTxData?: boolean): string { | |||
| signAndBuild(chainId: string, coinUsesNonPackedEncodingForTxData?: boolean): string { | |||
There was a problem hiding this comment.
If the API changes and is exposed publicly, it is a breaking change. Please mark the commit as such.
| @@ -38,6 +39,11 @@ export abstract class BaseNFTTransferBuilder { | |||
| throw new InvalidParameterValueError('Invalid expiration time'); | |||
| } | |||
|
|
|||
| walletVersion(version: number): this { | |||
There was a problem hiding this comment.
Nit: This function scheme sounds like a getter opposed to a setter. Maybe setWalletVersion?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket: WIN:2755
TICKET: WIN-2755