feat(sdk-coin-celo): support eip1559 for celo #5090
Merged
Conversation
This file contains hidden or 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
mullapudipruthvik
force-pushed
the
COIN-2046-support-eip1559-for-celo
branch
6 times, most recently
from
b26b94a to
94ea97b
Compare
| const tx = await txBuilder.build(); | ||
| const txJson = tx.toJson(); | ||
| should.equal(txJson.v, '0xaef3'); | ||
| should.equal(txJson.v, '0x015e09'); |
Contributor
Author
There was a problem hiding this comment.
after the hardfork, v value should be 2*chainId + 35
mullapudipruthvik
force-pushed
the
COIN-2046-support-eip1559-for-celo
branch
from
94ea97b to
a53163e
Compare
mullapudipruthvik
force-pushed
the
COIN-2046-support-eip1559-for-celo
branch
from
a53163e to
819193f
Compare
gianchandania
previously approved these changes
Nov 8, 2024
mullapudipruthvik
force-pushed
the
COIN-2046-support-eip1559-for-celo
branch
13 times, most recently
from
191184c to
945fb3c
Compare
| * | ||
| * @protected for internal use when the enableFinalVField flag is true. | ||
| */ | ||
| protected getFinalV(): string { |
Contributor
There was a problem hiding this comment.
This method is not needed for eip1559, right? Why are we adding this?
Contributor
Author
There was a problem hiding this comment.
this is just to unsure backward compatibility
I see testnet celo sort of support legacy as well https://alfajores.celoscan.io/tx/0x8c275e0c791d732cee3198c97ab8a54fd21b656a4a012857dddbd4c53a8e15ed
I'll test it out after release
| } | ||
|
|
||
| /** @inheritdoc */ | ||
| protected fromImplementation(rawTransaction: string): Transaction { |
Contributor
There was a problem hiding this comment.
This method is almost similar in the celo transaction builder, what is the reason to add this here?
Contributor
Author
There was a problem hiding this comment.
adarsh312
reviewed
Nov 21, 2024
| chainId: (coins.get('tcelo').network as EthereumNetwork).chainId, | ||
| }, | ||
| 'petersburg' | ||
| { hardfork: 'london' } |
Contributor
There was a problem hiding this comment.
should this be 'petersburg' ? how are we determining this ?
gianchandania
approved these changes
Nov 21, 2024
mullapudipruthvik
force-pushed
the
COIN-2046-support-eip1559-for-celo
branch
from
945fb3c to
3a7a64d
Compare
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: COIN-2046
TICKET: COIN-2046
Changes only to testnet