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

fuel core 0.13 upgrade #657

Merged
merged 20 commits into from
Nov 1, 2022
Merged

fuel core 0.13 upgrade #657

merged 20 commits into from
Nov 1, 2022

Conversation

Voxelot
Copy link
Member

@Voxelot Voxelot commented Oct 28, 2022

WIP to upgrade to fuel-core 0.13 with the new generic tx format

sway tests were compiled using forc from this PR: FuelLabs/sway#3181

Because there were breaking changes to the fuel-vm opcodes, we have to check in the compiled sway files for now until a new version of forc can be released. However, releasing forc is blocked on the SDK supporting fuel-core 0.13, so the upgrades were done concurrently.

@Voxelot
Copy link
Member Author

Voxelot commented Oct 28, 2022

Getting some strange errors from the tests - @xgreenx could you have a look?

Fixed almost all tests:
- 1 test fails `test_vector` with calling `array_in_vec` method.
- Increase gas limit or used gas in some tests, because we changed the gas calculation rules.
- Adapted signing for new cache rules.
@xgreenx
Copy link
Contributor

xgreenx commented Oct 28, 2022

Blocked by FuelLabs/fuel-vm#250 and FuelLabs/fuel-core#737

packages/fuels-core/Cargo.toml Outdated Show resolved Hide resolved
packages/fuels/tests/types.rs Outdated Show resolved Hide resolved
@Voxelot Voxelot marked this pull request as ready for review November 1, 2022 10:21
!packages/fuels/tests/**/Forc.toml
!packages/fuels/tests/**/Forc.lock
!packages/fuels/tests/.gitignore
# - name: Install Forc
Copy link
Member Author

Choose a reason for hiding this comment

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

commented out until forc can be released using this version of the sdk

@@ -78,8 +78,8 @@ jobs:
# install dasel
curl -sSLf "$DASEL_VERSION" -L -o dasel && chmod +x dasel
mv ./dasel /usr/local/bin/dasel
members=$(cat Cargo.toml | dasel -r toml -w json 'workspace.members' | jq -r ".[]" | xargs -L 1 basename | jq -R '[.]' | jq -s -c 'add')
echo "::set-output name=members::$members"
members=$(cat Cargo.toml | dasel -r toml -w json 'workspace.members' | jq -r ".[]" | xargs -I '{}' dasel -f {}/Cargo.toml 'package.name' | jq -R '[.]' | jq -s -c 'add')
Copy link
Member Author

Choose a reason for hiding this comment

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

fix cargo check to use actual package names, before each job was checking the whole project

@@ -117,14 +117,16 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha }}
Copy link
Member Author

Choose a reason for hiding this comment

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

fixes strange errors that appear when master is ahead of the current branch. By default this action will merge master with the PR on checkout, which can cause confusion when you encounter error messages related to code changes not on the current branch.

@@ -135,10 +137,10 @@ jobs:

- name: Install WebAssembly Test harness
if: ${{ matrix.command == 'test' }}
uses: actions-rs/cargo@v1
uses: baptiste0928/cargo-install@v1
Copy link
Member Author

Choose a reason for hiding this comment

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

takes ~1s with a warm cache instead of ~5m

with:
name: sway-examples
path: packages/fuels/tests/
# - name: Download sway example artifacts
Copy link
Member Author

Choose a reason for hiding this comment

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

commented out until forc can be released using this version of the sdk

hal3e
hal3e previously requested changes Nov 1, 2022
Copy link
Contributor

@hal3e hal3e left a comment

Choose a reason for hiding this comment

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

You have commented a line in the .gitignore and now you are pushing the outputs of the sway projects.

@@ -16,7 +16,7 @@ docs/book/
packages/fuels/tests/**/**/Forc.lock

# Don't add out/ files from test Sway projects.
packages/fuels/tests/**/**/out/
# packages/fuels/tests/**/**/out/
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO you need to remove this.

Copy link
Member Author

@Voxelot Voxelot Nov 1, 2022

Choose a reason for hiding this comment

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

This was intentional.

sway tests were compiled using forc from this PR: FuelLabs/sway#3181

We can't get these sway files to work with the new VM without this version of forc. However forc can't be released until the SDK is published with fuel-core v0.13 support.

See this thread for more context: https://fuellabs.slack.com/archives/C02RU9MNVKN/p1666934602020589

@@ -111,9 +116,9 @@ impl Provider {
&self,
tx: &Transaction,
) -> Result<(TransactionStatus, Vec<Receipt>), ProviderError> {
let tx_id = self.client.submit(tx).await?.0.to_string();
let tx_id = tx.id().to_string();
let status = self.client.submit_and_await_commit(tx).await?;
Copy link
Member Author

Choose a reason for hiding this comment

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

Since exeuction is no longer synchronous with tx submission, this new method will poll on the client until the tx has been processed into a block or rejected by the tx pool.

@@ -5,3 +5,5 @@ license = "Apache-2.0"
name = "auth_testing_abi"

[dependencies]
core = { git = "https://github.com/FuelLabs/sway", branch = "Voxelot/fuel-core-0.13" }
Copy link
Member Author

Choose a reason for hiding this comment

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

this was needed to force forc to use the updated sway libs instead of the older versions on the latest release tag

!packages/fuels/tests/**/Forc.toml
!packages/fuels/tests/**/Forc.lock
!packages/fuels/tests/.gitignore
# - name: Install Forc
Copy link
Member Author

Choose a reason for hiding this comment

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

commented out until forc can be released using this version of the sdk

xgreenx
xgreenx previously approved these changes Nov 1, 2022
@digorithm
Copy link
Member

digorithm commented Nov 1, 2022

@FuelLabs/sdk-rust

We're pushing forc outputs temporarily until the next releases of the other components due to some circular dependency issues!

Once those are out we can remove these binaries/JSONs and uncomment the CI-related stuff!

digorithm
digorithm previously approved these changes Nov 1, 2022
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

Tracking the clean-up task here: #664.

@xgreenx xgreenx dismissed stale reviews from digorithm and themself via fa1578d November 1, 2022 17:40
Comment on lines -70 to -107
impl From<MessageConfig> for Message {
fn from(msg: MessageConfig) -> Self {
Message {
sender: msg.sender,
recipient: msg.recipient,
nonce: msg.nonce,
amount: msg.amount,
data: msg.data,
da_height: DaBlockHeight(msg.da_height),
fuel_block_spend: None,
}
}
}

#[skip_serializing_none]
#[serde_as]
#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
pub struct CoinConfig {
#[serde_as(as = "Option<HexType>")]
#[serde(default)]
pub tx_id: Option<Bytes32>,
#[serde_as(as = "Option<HexNumber>")]
#[serde(default)]
pub output_index: Option<u64>,
#[serde_as(as = "Option<HexNumber>")]
#[serde(default)]
pub block_created: Option<BlockHeight>,
#[serde_as(as = "Option<HexNumber>")]
#[serde(default)]
pub maturity: Option<BlockHeight>,
#[serde_as(as = "HexType")]
pub owner: Address,
#[serde_as(as = "HexNumber")]
pub amount: u64,
#[serde_as(as = "HexType")]
pub asset_id: AssetId,
}

Copy link
Member

Choose a reason for hiding this comment

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

🔥

@Voxelot Voxelot merged commit f79b458 into master Nov 1, 2022
@Voxelot Voxelot deleted the Voxelot/fuel-core-0.13 branch November 1, 2022 22:15
MujkicA pushed a commit that referenced this pull request Nov 4, 2022
WIP to upgrade to fuel-core 0.13 with the new generic tx format

sway tests were compiled using forc from this PR:
FuelLabs/sway#3181

Because there were breaking changes to the fuel-vm opcodes, we have to
check in the compiled sway files for now until a new version of forc can
be released. However, releasing forc is blocked on the SDK supporting
fuel-core 0.13, so the upgrades were done concurrently.

Co-authored-by: green <xgreenx9999@gmail.com>
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.

None yet

4 participants