fix: calculate transaction fee

[Torres-ssf]

Pretty much what the title says.

The calculateTransactionFee method was updated based on the version used by fuels-wallet

[github-actions]

Coverage report

St.❔	Category	Percentage	Covered / Total
🟢	Statements	84.56% (+0.02% 🔼)	4677/5531
🟡	Branches	63.46% (-0.5% 🔻)	639/1007
🟡	Functions	68.6% (+0.36% 🔼)	745/1086
🟢	Lines	84.68% (-0% 🔻)	4488/5300

Show files with reduced coverage 🔻

St.❔	File	Statements	Branches	Functions	Lines
🔴	... / provider.ts	50.51% (-0.51% 🔻)	42.86%	48.08%	51.83% (-0.27% 🔻)
🔴	... / transaction-response.ts	20.51% (-1.11% 🔻)	0%	0%	20.51% (-1.11% 🔻)

Test suite run success

1073 tests passing in 179 suites.

Report generated by 🧪jest coverage report action from 468e3dc

[arboleya]

Firstly, I believe it came from @camsjams Vector PR, but just in case, I will mention it here — there are a couple of lint annotations:

• https://github.com/FuelLabs/fuels-ts/actions/runs/5575462945

Lint errors are like: xyz is defined but never used

Finally, following our last fuel-gauge discussion about where to put tests and whatnot, should we move the fee.test.ts over there (to fuel-gauge) since it's not a unit test?

[Torres-ssf]

Firstly, I believe it came from @camsjams Vector PR, but just in case, I will mention it here — there are a couple of lint annotations:

• https://github.com/FuelLabs/fuels-ts/actions/runs/5575462945

Lint errors are like: xyz is defined but never used

Finally, following our last fuel-gauge discussion about where to put tests and whatnot, should we move the fee.test.ts over there (to fuel-gauge) since it's not a unit test?

@arboleya but the tests inside fee.test.ts are unit tests 🤔

@nedsalk just suggested to make them feature test, so instead of having these unit tests on fee.test.ts, we would have the value returned by these functions validated in the test that tests the function that makes the call to calculateTransactionFee

[arboleya]

@Torres-ssf Are they unit tests? I thought you were querying the node. Perhaps I got something wrong.

[danielbate]

@arboleya LGTM as the tests in that file are the calculations that are passed values that would be from the node. Begs the question as to whether we'd want an E2E that tests for a specific gasUsed and fee value for a given contract. At a glance most of our tests check that gasUsed or fee are at least defined. I don't think such a test is necessary for this piece of work so will leave up to @Torres-ssf 👍🏻

[arboleya]

@Torres-ssf @danielbate My bad, I mixed things up.

The fee.test.ts is indeed a unit.

Unlike the provider.test.ts — which uses the node for fetching stuff.

As I said before, I prefer this approach of having the /test directory, but that wasn't what we discussed at our last discussion with @camsjams. From what I recall, this test should be on the fuel-gauge and import the Provider from fuels, like so:

import { Provider } from 'fuels';

Was it me who understood things wrong? I'm not claiming I'm right.

But we surely need to clearly define these convention rules to avoid misinterpretations.

[danielbate]

My understanding is that unit tests can remain and will be alongside src files. Feature tests, which will test the entry points for the packages will go in a /test directory. Anything making calls to a node and thus requiring encoding/decoding etc will go in fuel-gauge as an integration test. Of which should be prioritised and implemented is still under discussion / up to the implementer.

I plan on defining this more clearly in #1043 once #1079 has been completed and merged.

Apologies also I missed the provider.test.ts, agreed that under definition this would come under an integration test and should be within fuel-gauge.

[Torres-ssf]

@arboleya @danielbate I've only touched on one test case from provider.test.ts to make it conform with the changes that I made.

No test cases were implemented on it.

Do you think that moving provider.test.ts to fuel-gauge should be part of this PR scope?

[danielbate]

@Torres-ssf I'd be happy to count it as out of scope of this work and instead include it in a provider test refactor piece

[arboleya]

[Torres-ssf]

@arboleya @danielbate I've just revet the renaming of a method and query to avoid conflicts with the fuels wallet