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

fix: optionalMessage and optionalGas encoding #536

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

saadahmsiddiqui
Copy link
Member

Description

optionalGas when set to 0 is not encoded by evm SDK. This PR fixes the encoding issue

Related Issue Or Context

#535

Closes: #535

How Has This Been Tested? Testing details.

  • Add encoding unit test

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas.
  • I have ensured that all acceptance criteria (or expected behavior) from issue are met
  • I have updated the documentation locally and in chainbridge-docs.
  • I have added tests to cover my changes.
  • I have ensured that all the checks are passing and green, I've signed the CLA bot

@LyonSsS
Copy link

LyonSsS commented Sep 12, 2024

Tested on PR - > deposits with value "0" for optionalGas now is encoded correct
https://sepolia.etherscan.io/tx/0x8cad649a671cd033ca02071c38cbadfbf40dc42f289e06cc242c97d6e6c3e223
https://sepolia.etherscan.io/tx/0xc11821a70128cbef017906314cf3d311343c5f57135e76be09f52ac8b8c61016

@saadahmsiddiqui saadahmsiddiqui merged commit ebc8885 into main Sep 12, 2024
3 checks passed
@saadahmsiddiqui saadahmsiddiqui deleted the fix/zero-optional-gas branch September 12, 2024 13:27
saadahmsiddiqui pushed a commit that referenced this pull request Sep 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.3.1](evm-v1.3.0...evm-v1.3.1)
(2024-09-12)


### Bug Fixes

* `optionalMessage` and `optionalGas` encoding
([#536](#536))
([ebc8885](ebc8885))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

"0" set to OptionalGas in ERC20+Contract call deposit, will be skipped in encoding
3 participants