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(CLI): use aliases to avoid needlessly redeploying contracts #88

Merged
merged 23 commits into from
Jul 13, 2024

Conversation

BlaineHeffron
Copy link
Contributor

@BlaineHeffron BlaineHeffron commented Jun 24, 2024

Resolves issue #73 (comment)

@BlaineHeffron BlaineHeffron marked this pull request as ready for review June 25, 2024 17:36
@BlaineHeffron BlaineHeffron marked this pull request as draft June 25, 2024 19:25
@BlaineHeffron BlaineHeffron marked this pull request as ready for review June 26, 2024 15:52
@BlaineHeffron BlaineHeffron force-pushed the feat/improve-handle-contracts branch from 27ad6c7 to 2beed2d Compare June 26, 2024 15:53
Copy link
Contributor

@chadoh chadoh left a comment

Choose a reason for hiding this comment

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

Looks great! Love the refactoring. Just one (one and a half?) quick cleanup item(s).

crates/loam-cli/src/commands/build/clients.rs Outdated Show resolved Hide resolved
crates/loam-cli/src/commands/build/clients.rs Outdated Show resolved Hide resolved
@BlaineHeffron BlaineHeffron requested a review from willemneal June 26, 2024 16:14
Copy link
Contributor

@chadoh chadoh left a comment

Choose a reason for hiding this comment

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

We need tests!

@BlaineHeffron
Copy link
Contributor Author

Added tests - adds it twice, ensure alias check works. Changes the file, ensures it replaces. Changes again with production, ensures error is thrown.

@BlaineHeffron BlaineHeffron requested a review from chadoh June 27, 2024 23:40
@BlaineHeffron BlaineHeffron marked this pull request as draft June 28, 2024 06:08
@BlaineHeffron
Copy link
Contributor Author

Tests are working on moss but when run on github servers its getting this error:

 📲 installing "hello_world" wasm bytecode on-chain...
error: transaction submission failed: Some(
    TransactionResult {
        fee_charged: 40326,
        result: TxFailed(
            VecM(
                [
                    OpInner(
                        InvokeHostFunction(
                            ResourceLimitExceeded,
                        ),
                    ),
                ],
            ),
        ),
        ext: V0,
    },
)

Not sure what is going on

@willemneal
Copy link
Collaborator

THis is a flaky test issue that appears in the CLI too. The solution I had was to add sleeps to the failing test. However, this is not the correct solution for sure. One possible reason is that the tests are all trying to install and deploy the same contract. So one solution is to add random/unique data to the wasm's custom section. This will make the wasm have a new hash. So as long as the data that is used in the custom section is the same a test than the hash check you are doing should still work.

We can chat about it if you want. I need to do it for the CLI so would be good to figure out an easy way to do it. Maybe we can make a small crate and publish to crates.io. This way we can use it across both projects and it'll be a lesson in publishing crates!

@BlaineHeffron
Copy link
Contributor Author

Tried @willemneal suggestion, didn't seem to work. Very strange, makes me wonder if there is a different underlying issue here.

@BlaineHeffron BlaineHeffron force-pushed the feat/improve-handle-contracts branch from 89f96b7 to 74d8aa2 Compare July 12, 2024 12:21
@BlaineHeffron
Copy link
Contributor Author

Turns out Willem's suggestion worked, problem was I modified the wrong wasm file (the one in the build directory instead of the one copied to /target/loam).

@BlaineHeffron BlaineHeffron marked this pull request as ready for review July 13, 2024 02:19
@chadoh chadoh changed the title check if alias exists and if it would be duplicated before deploying … feat(CLI): use aliases to avoid needlessly redeploying contracts Jul 13, 2024
@chadoh chadoh enabled auto-merge (squash) July 13, 2024 02:22
Copy link
Contributor

@chadoh chadoh left a comment

Choose a reason for hiding this comment

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

Hooray!

@chadoh chadoh merged commit 460b351 into main Jul 13, 2024
2 checks passed
@chadoh chadoh deleted the feat/improve-handle-contracts branch July 13, 2024 02:22
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