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.

Sign up for GitHub

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

Jump to bottom

feat: add message support #515

Merged
merged 17 commits into from
Sep 27, 2022
Merged

feat: add message support #515

merged 17 commits into from
Sep 27, 2022

Conversation

camsjams
Copy link
Contributor

Closes #504 and

Closes #513 (just used newer Docker image to cover for this)

@camsjams camsjams requested a review from a team September 22, 2022 08:01
@camsjams camsjams self-assigned this Sep 22, 2022
Cameron Manavian added 2 commits September 22, 2022 01:05
camsjams
camsjams commented Sep 22, 2022
View reviewed changes
packages/providers/src/provider.ts Outdated Show resolved Hide resolved
packages/providers/src/transaction-request/input.ts Show resolved Hide resolved
packages/transactions/src/coders/input.ts Show resolved Hide resolved
packages/transactions/src/coders/input.ts Show resolved Hide resolved
services/fuel-core/Dockerfile Show resolved Hide resolved
services/fuel-core/Dockerfile
@@ -21,7 +22,6 @@ CMD exec ./fuel-core run \
--ip ${IP} \
--port ${PORT} \
--db-path ${DB_PATH} \
--utxo-validation \
--min-gas-price ${MIN_GAS_PRICE} \
Copy link
Contributor Author

@camsjams camsjams Sep 22, 2022 •
edited

Choose a reason for hiding this comment

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

removing --utxo-validation: this is probably a no-go?

I need to figure out why this flag breaks the message id check on fuel-core

But all the tests pass here on GitHub with this flag disabled, so maybe we can keep it turned off

Copy link
Member

Choose a reason for hiding this comment

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

I think if it works without could be an issue once we have some predicate tests that would require this to be enabled. @Voxelot do you have some idea why this could be happening?

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2022 •
edited

Coverage report

St.❔
 Category Percentage Covered / Total
🟢 Statements
89.2% (-0.42% 🔻)
 3443/3860
🟡 Branches
70.19% (+0.18% 🔼)
 657/936
🟢 Functions
86.54% (+0.33% 🔼)
 688/795
🟢 Lines
89.13% (-0.45% 🔻)
 3296/3698
Show files with reduced coverage 🔻
St.❔
 File Statements Branches Functions Lines
🟢
... / input.ts
80.82% (-16.91% 🔻)
70% (-5% 🔻)
93.33% (-6.67% 🔻)
80.82% (-16.91% 🔻)
🟢
... / wallet.ts
94.39% (-0.45% 🔻)
76.19% (+1.19% 🔼)
96% (+0.17% 🔼)
94.34% (-0.45% 🔻)

Test suite run success

520 tests passing in 46 suites.

Report generated by 🧪jest coverage report action from 119955e

luizstacio
luizstacio requested changes Sep 22, 2022
View reviewed changes
packages/contract/src/__test__/coverage-contract/coverage-contract.test.ts Outdated Show resolved Hide resolved
packages/transactions/src/coders/input.ts Show resolved Hide resolved
packages/contract/src/__test__/coverage-contract/coverage-contract.test.ts Outdated Show resolved Hide resolved
packages/transactions/src/coders/input.ts Show resolved Hide resolved
services/fuel-core/Dockerfile
@@ -21,7 +22,6 @@ CMD exec ./fuel-core run \
--ip ${IP} \
--port ${PORT} \
--db-path ${DB_PATH} \
--utxo-validation \
--min-gas-price ${MIN_GAS_PRICE} \
Copy link
Member

Choose a reason for hiding this comment

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

I think if it works without could be an issue once we have some predicate tests that would require this to be enabled. @Voxelot do you have some idea why this could be happening?

packages/providers/src/provider.ts Outdated Show resolved Hide resolved
packages/transactions/src/coders/input.test.ts Show resolved Hide resolved
Cameron Manavian and others added 3 commits September 26, 2022 07:50
@luizstacio
Update packages/wallet/src/wallet.ts
730255c 
Co-authored-by: Luiz Estácio | stacio.eth <luizstacio@gmail.com>
@luizstacio
Update packages/contract/src/__test__/coverage-contract/coverage-cont…
a883cb8 
…ract.test.ts

Co-authored-by: Luiz Estácio | stacio.eth <luizstacio@gmail.com>
Merge branch 'master' into cm/issue-504
cf83879
@camsjams camsjams enabled auto-merge (squash) September 26, 2022 14:51
@camsjams camsjams requested review from luizstacio and a team September 26, 2022 14:51
@Voxelot
Copy link
Member

Voxelot commented Sep 26, 2022 •
edited

I think if it works without could be an issue once we have some predicate tests that would require this to be enabled. @Voxelot do you have some idea why this could be happening?

@luizstacio This could be related to FuelLabs/fuel-tx#186

@camsjams camsjams merged commit fa83fcd into master Sep 27, 2022
@camsjams camsjams deleted the cm/issue-504 branch September 27, 2022 01:27
@github-actions github-actions bot mentioned this pull request Sep 27, 2022
Merged
@luizstacio luizstacio mentioned this pull request Oct 13, 2022
Closed
@camsjams camsjams mentioned this pull request Nov 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuinnLee QuinnLee QuinnLee approved these changes

@luizstacio luizstacio luizstacio approved these changes

@LuizAsFight LuizAsFight Awaiting requested review from LuizAsFight

Assignees

@camsjams camsjams

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use new Message specs once fuel-core update comes out Add message support
4 participants
@camsjams @Voxelot @QuinnLee @luizstacio