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

Upgraded CosmWasm runtime in Picasso to latest dependencies #3677

Merged
merged 7 commits into from
Jun 12, 2023

Conversation

dzmitry-lahoda
Copy link
Contributor

@dzmitry-lahoda dzmitry-lahoda commented Jun 9, 2023

  • allowed to run parity and cosmos rust code in same wasm contract and fancy staff for precompiles
  • updated host with host call depth no more cw deps reexport cosmwasm-vm#36
  • removed reexport of cw std
  • updated cw to version 1.2.+
  • updated XCVM spec with interpetere != account, removed autogenerated obsolete ToC, made more clear impl contracts vs logic , removed mosaic died long ago
  • renamed XCVM to XC as XCVM long and hard to type, and really it is more about xc/swaps/mev rather than VM, which uses some VMs
  • made features, std, no_std, right as i could
  • forks will be replaced by forks into composablefi under <name>-no-std later . all forks are PRs and issues on CW team repos
  • unified deps into workspace deps
  • started to use cw_serde fancy API to CW
  • added XC to CI
  • XCVM tests where failing with cargo-package-build> thread 'tests::suite::cross_chain::test_simple_crosschain_xcvm_transfer' panicked at 'Must be able which was before my commit anyway
  • go over CW pallet guide - contracts work on Picasso

image

  • PR title is my best effort to provide summary of changes and has clear text to be part of release notes
  • I marked PR by misc label if it should not be in release notes
  • I have linked Zenhub/Github or any other reference item if one exists
  • I was clear on what type of deployment required to release my changes (node, runtime, contract, indexer, on chain operation, frontend, infrastructure) if any in PR title or description
  • I waited and did best effort for pr-workflow-check / draft-release-check to finish with success(green check mark) with my changes
  • I have added at least one reviewer in reviewers list
  • I tagged(@) or used other form of notification of one person who I think can handle best review of this PR
  • I have proved that PR has no general regressions of relevant features and processes required to release into production

@github-actions
Copy link

github-actions bot commented Jun 9, 2023

@dzmitry-lahoda dzmitry-lahoda changed the title Upgraded Cosmwasm to latest dependencies Upgraded CosmWasm runtime in Picasso to latest dependencies Jun 9, 2023
@dzmitry-lahoda dzmitry-lahoda requested review from mina86 and kkast June 9, 2023 18:30
@mina86
Copy link
Contributor

mina86 commented Jun 9, 2023

This should be split into multiple PRs. It’s quite hard to review as it mixes multiple changes into one. For example, XCVM → XC rename could easily be done in a separate PR. I’ve done cursory read through and so far looks good but haven’t reviewed it properly yet.

@dzmitry-lahoda
Copy link
Contributor Author

all changes are really easy, very stupid pr just doing cargo staff, some rename and clean up. what else should i do waiting for rust to compile? )

code/parachain/frame/cosmwasm/Cargo.toml Show resolved Hide resolved
code/xcvm/cosmwasm/tests/src/tests/suite.rs Outdated Show resolved Hide resolved
code/xcvm/lib/core/src/cosmwasm.rs Outdated Show resolved Hide resolved
code/xcvm/lib/core/src/cosmwasm.rs Outdated Show resolved Hide resolved
@mina86
Copy link
Contributor

mina86 commented Jun 9, 2023

Each individual change may be easy, but once you package them into a single PR they all produce big PR which is hard to read since each diff line corresponds to different thing being changed. You could have for example rename XCVM to XC, send that as a PR, and continue working on the next thing even on the same local branch.

@dzmitry-lahoda dzmitry-lahoda requested a review from mina86 June 10, 2023 12:01
@dzmitry-lahoda
Copy link
Contributor Author

send that as a PR, and continue working on the next thing even on the same local branch.

as soon as somebody proves it works with rebases - sure. will do exactly that

@mina86
Copy link
Contributor

mina86 commented Jun 12, 2023

as soon as somebody proves it works with rebases - sure. will do exactly that

What do you mean proves it works with rebases? I’ve been using rebases entire time I've been using git and they work great.

@dzmitry-lahoda
Copy link
Contributor Author

dzmitry-lahoda commented Jun 12, 2023

What do you mean proves it works with rebases? I’ve been using rebases entire time I've been using git and they work great.

@mina86 show me you operated like you said - i would happy to follow. i have not seen you do PR, and than another PR on top of PR from same branch. like first PR into main. second PR into first PR. etc

I feel it does not work well in our context, but I may be wrong.

@dzmitry-lahoda dzmitry-lahoda added this pull request to the merge queue Jun 12, 2023
Merged via the queue into main with commit e09e0b3 Jun 12, 2023
@dzmitry-lahoda dzmitry-lahoda deleted the dz/224 branch June 12, 2023 14:11
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.

3 participants