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: transaction estimation for InputMessage containing data #3068

Closed
wants to merge 9 commits into from

Conversation

LuizAsFight
Copy link
Contributor

@LuizAsFight LuizAsFight commented Aug 30, 2024

Release notes

In this release, we:

  • Fixed tx estimation when InputMessage contains data

Summary

Messages with data cannot fund transactions.

This would happen in Fuel accounts that received a deposit from an ERC20 token (like in the Bridge).

Checklist

  • All changes are covered by tests (or not applicable)
  • All changes are documented (or not applicable)
  • I reviewed the entire PR myself (preferably, on GH UI)
  • I described all Breaking Changes (or there's none)

Copy link

vercel bot commented Aug 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
create-fuels-template ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 31, 2024 9:48am
ts-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 31, 2024 9:48am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
create-fuels-counter-example ⬜️ Ignored (Inspect) Aug 31, 2024 9:48am

@github-actions github-actions bot added the chore Issue is a chore label Aug 30, 2024
Copy link
Contributor

github-actions bot commented Aug 30, 2024

This PR is published in NPM with version 0.0.0-pr-3068-20240831093013

@arboleya arboleya changed the title chore: fix TX estimation chore: fix transaction estimation Aug 31, 2024

// We only consider the message input if it has no data.
// Messages with `data` cannot fund the gas of a transaction.
if (input.type === InputType.Message && bn(input.data).isZero()) {
Copy link
Member

Choose a reason for hiding this comment

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

Adding a test covering this would be great, and then we're good to go.

cc @Torres-ssf @petertonysmith94

Copy link
Contributor

Choose a reason for hiding this comment

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

@Torres-ssf I assume as you've self assigned you'll add the test for this?

@arboleya arboleya changed the title chore: fix transaction estimation fix: transaction estimation for InputMessage containing data Aug 31, 2024
@arboleya arboleya added bug Issue is a bug and removed chore Issue is a chore labels Aug 31, 2024
@github-actions github-actions bot added the chore Issue is a chore label Aug 31, 2024
Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
79.55%(+0%) 71.85%(-0.06%) 78%(+0%) 79.68%(+0%)
Changed Files:
Ok File (✨=New File) Lines Branches Functions Statements
🔴 packages/account/src/account.ts 82.71%
(-0.11%)
62.66%
(-1.72%)
86.84%
(+0%)
82.53%
(+0%)

@Torres-ssf
Copy link
Contributor

Closing in favor of #3078

@Torres-ssf Torres-ssf closed this Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error on contract call forwarding an assetId different from base assetId
4 participants