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

Feat: Add Injective Proto Paths Using Build Flags #10

Closed

Conversation

maxrobot
Copy link

@maxrobot maxrobot commented Jan 21, 2025

Description

In this PR the following changes are made:

  • Update the optimizer version to 0.16.1
  • Update the protos to support Injective tokenfactory
  • Change the type of igp to Uint128 as u128 is no longer supported.
  • Ignore tests that don't work (note I think the ethereum ABIs are outdated...)
  • Added labels to contracts in tests as None is not supported in some minor version that is being pulled (hence Cargo.lock)

Now when the optimizer is run it creates two binaries for the warp native contract:

  • Osmosis: with the standard tokenfactory proto path
  • Injective: with the injective tokenfactory proto path

Everything else is the same.

Note:

Please commit the Cargo.lock in future as I think it will make it easier for others to successfully build and run the tests.

Comment on lines +21 to +22
- rename
- foundry
Copy link
Member

Choose a reason for hiding this comment

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

were these just undocumented? I dont see them referenced in this PR

Copy link
Author

Choose a reason for hiding this comment

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

If you try to run the make scripts and don't have these then it fails:

optimize-fast:
	cargo cw-optimizoor
	rename --force 's/(.*)-(.*)\.wasm/$$1\.wasm/d' artifacts/*

and if you haven't install foundry then the wasm integrations tests don't work as it needs anvil...

Copy link
Member

Choose a reason for hiding this comment

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

should these changes be reverted?

Copy link
Author

Choose a reason for hiding this comment

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

So if you don't have these defined and run optimize-fast the tests fail.

pub default_gas_usage: u128,
pub default_gas_usage: Uint128,
Copy link
Member

Choose a reason for hiding this comment

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

curious why this was necessary
we use u128 elsewhere too

Copy link
Author

Choose a reason for hiding this comment

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

If you take a clean repo and run the tests you will notice this will fail.

This is an issue because you didn't commit Cargo.lock so all the dependencies will pull the latest patch version.

Copy link
Member

Choose a reason for hiding this comment

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

looks mostly good to me although maybe could have just had the feature flag affect the URL prefix

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.

2 participants