-
Notifications
You must be signed in to change notification settings - Fork 27
chore: Merge v1.0.x changes in to development #494
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
Conversation
* Updated ownership process * Added expiring ownership offer * Linting * Fixed failing test
…nd removed .clone() from it
add link to andr.js repo
* replaced Milliseconds with either MillisecondsDuration or MillisecondsExpiration * Splitter struct's lock comment improvement and change to MillisecondsExpiration
* added valid_cw20_contract in auction's state, adjusted validate_denom to accept the valid cw20 contract * execute_place_bid_cw20 for auction * added support for cw20 in auction's execute_claim * returned the original order of messages in the response * cw20 support for claim, adjusted validate_denom, use permissioning system to set valid cw20 * added unit tests for cw20 support * renamed cw20hookmsg's purchase to PlaceBid * updated auction's schema * changed ReceiveCw20 to Receive, integration test for auction with cw20 * updated auction's schema * additional checks for auction_cw20 integration test * improved error message for InvalidFunds * unit test adjustment for new error message * moved denom_validation to denom file in the common folder for easier implementation for other contracts * added uses_cw20 in auction state * cw20 support for marketplace ADO * added unit tests for cw20 support in Marketplace * added integration test for cw20-enabled marketplace, adjusted tax payments when buying with cw20 * stopped sending 0 funds when tax recipient and amount are None in Marketplace * improved some error messages * added denom validation for crowdfund * changed whitelist to in AuctionState to a bool, validate whitelist in auction using permissioning system, whitelist is set per individual auction * implemented new whitelist system to update_auction * adjustments to denom.rs, auction, marketpalce, splitter and crowdfund * fixed cw20 tax payments in auction, same implementation as marketplace * fmt fix * added recipient's address support for cw20 auction * removed check that prevented open auctions * added conditional statement for after tax payment in marketplace * resolves #402 * added recipient address support for cw20 sales in Marketpalce * added recipient to cw20 auction integration test * added whitelist testing in auction integration test * implemented genereate_msg_cw20 in marketplace and auction * refactor: remove unncessary checks, improve error message, add same check for denom validation in update auction as in start auction
* added lower fund limit to the create batch * added validation for maximum funds for batch creation * fixed linting err * fixed validation process error * updated test case * feat: Added MockVestingContract struct to Vesting Contract * fix: Removed unused file --------- Co-authored-by: Connor Barr <crnbarr@gmail.com>
…408) * fix: Replaced Expiration with MillisecondsExpiration in UpdateOwner Old messages will be incompatible with this change if they have an expiration field * fix: Replaced Expiration with MillisecondsExpiration in permissioning messages Old permissioning messages will be incompatible if they contained an expiration
fix link and change pic reference to the PR branch
* fix: instantiate2 injective error * Bump version for app-contract and app-package
WalkthroughThe recent updates provide a comprehensive set of enhancements and clean-ups across multiple project components. Significant new content was added to the Changes
Sequence Diagram(s)(Skip - no new features or notable modifications to control flow) Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
Cargo.lock
is excluded by!**/*.lock
asset/Andromeda-IBC.png
is excluded by!**/*.png
Files selected for processing (18)
- README.md (3 hunks)
- contracts/app/andromeda-app-contract/Cargo.toml (1 hunks)
- contracts/data-storage/andromeda-primitive/Cargo.toml (1 hunks)
- contracts/finance/andromeda-rate-limiting-withdrawals/Cargo.toml (1 hunks)
- contracts/fungible-tokens/andromeda-cw20-staking/Cargo.toml (1 hunks)
- contracts/fungible-tokens/andromeda-cw20-staking/src/contract.rs (2 hunks)
- contracts/fungible-tokens/andromeda-cw20/Cargo.toml (1 hunks)
- contracts/fungible-tokens/andromeda-lockdrop/Cargo.toml (1 hunks)
- contracts/non-fungible-tokens/andromeda-auction/Cargo.toml (1 hunks)
- contracts/non-fungible-tokens/andromeda-auction/src/contract.rs (3 hunks)
- contracts/non-fungible-tokens/andromeda-auction/src/testing/tests.rs (1 hunks)
- contracts/non-fungible-tokens/andromeda-crowdfund/Cargo.toml (1 hunks)
- contracts/non-fungible-tokens/andromeda-marketplace/src/testing/tests.rs (1 hunks)
- contracts/os/andromeda-vfs/Cargo.toml (1 hunks)
- git-conventional-commits.yaml (1 hunks)
- packages/andromeda-app/src/app.rs (1 hunks)
- packages/andromeda-data-storage/Cargo.toml (1 hunks)
- packages/std/src/ado_contract/execute.rs (1 hunks)
Files skipped from review due to trivial changes (13)
- contracts/app/andromeda-app-contract/Cargo.toml
- contracts/data-storage/andromeda-primitive/Cargo.toml
- contracts/finance/andromeda-rate-limiting-withdrawals/Cargo.toml
- contracts/fungible-tokens/andromeda-cw20-staking/Cargo.toml
- contracts/fungible-tokens/andromeda-cw20-staking/src/contract.rs
- contracts/fungible-tokens/andromeda-cw20/Cargo.toml
- contracts/fungible-tokens/andromeda-lockdrop/Cargo.toml
- contracts/non-fungible-tokens/andromeda-auction/Cargo.toml
- contracts/non-fungible-tokens/andromeda-auction/src/testing/tests.rs
- contracts/non-fungible-tokens/andromeda-crowdfund/Cargo.toml
- contracts/os/andromeda-vfs/Cargo.toml
- git-conventional-commits.yaml
- packages/andromeda-data-storage/Cargo.toml
Additional context used
LanguageTool
README.md
[style] ~23-~23: Consider using the synonym “brief” (= concise, using a few words, not lasting long) to strengthen your wording.
Context: ...itecture of the system to develop.A quick description of each of the components t...
(QUICK_BRIEF)
[style] ~28-~28: In American English, abbreviations like “etc.” require a period.
Context: ...or referencing ADOs, services, network, etc
- [aOS Economics](https://docs.androm...
(ETC_PERIOD)
[uncategorized] ~50-~50: Possible missing comma found.
Context: ...ild Applications that span accross many chains taking advantage of all the benefits th...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~52-~52: Did you mean “too Injective to”?
Context: ...leverage some protocol and another part to Injective to leverage some functionality there. The ...(TOO_ADJECTIVE_TO)
[style] ~60-~60: Consider a shorter alternative to avoid wordiness.
Context: ...d need to hire a full development teams in order to build their projects and custom smart ...(IN_ORDER_TO_PREMIUM)
[style] ~60-~60: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...omeda eliminates this need by providing a very large amount of custom smart contracts that u...(EN_WEAK_ADJECTIVE)
[uncategorized] ~66-~66: Possible missing comma found.
Context: ...s it stands, the ALL contains around 25 ADOs which is the tip of the iceberg. More a...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~74-~74: Specify a number, remove phrase, or simply use “many” or “numerous”
Context: ...rs access to the following:
- A large number of production ready ADOs to be used. ...
(LARGE_NUMBER_OF)
[duplication] ~78-~78: Possible typo: you repeated a word
Context: ...in that implements the aOS will be able to to see where aOS is also deployed and migh...(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~86-~86: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...lassified based on their functionality. Currently we have 7 different contract categories...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[grammar] ~91-~91: Consider using either the past participle “allowed” or the present participle “allowing” here.
Context: ...ntracts/ecosystem) | Contracts that are allow interaction with different ecosystem pr...(BEEN_PART_AGREEMENT)
[style] ~105-~105: The preposition ‘amongst’ is correct, but some people think that it is old-fashioned or literary. A more frequently used alternative is the preposition “among”.
Context: ...| Contract used to split any sent funds amongst defined addresses. | [Gitbook](https:/...(AMONGST)
[grammar] ~113-~113: Two determiners in a row. Choose either “an” or “a”.
Context: ...ontract that can receive an NFT and run an a sale on it.| [Gitbook](https://docs.and...(DT_DT)
[uncategorized] ~190-~190: A different word order might sound more natural.
Context: ...o fmt### Linting You should run `clippy` also. This is a lint tool for rust. It sugge... (AI_HYDRA_LEO_WORD_ORDER) --- [typographical] ~207-~207: Consider adding a comma after ‘Usually’ for more clarity. Context: ... deployed on many of the Cosmos chains. Usually this will require you to set up an envi... (RB_LY_COMMA) --- [grammar] ~207-~207: It seems that hyphens are missing. Context: ...dromeda has built the Andromeda CLI, an all in one tool to build, interact, and manage ADO... (ALL_IN_ONE_HYPHEN) </blockquote></details> </blockquote></details> <details> <summary>Markdownlint</summary><blockquote> <details> <summary>README.md</summary><blockquote> 12-12: Expected: h2; Actual: h3 Heading levels should only increment by one level at a time (MD001, heading-increment) --- 84-84: Expected: 0 or 2; Actual: 1 Trailing spaces (MD009, no-trailing-spaces) --- 88-88: Expected: 0 or 2; Actual: 129 Trailing spaces (MD009, no-trailing-spaces) --- 91-91: Expected: 0 or 2; Actual: 22 Trailing spaces (MD009, no-trailing-spaces) --- 92-92: Expected: 0 or 2; Actual: 86 Trailing spaces (MD009, no-trailing-spaces) --- 103-103: Expected: 0 or 2; Actual: 6 Trailing spaces (MD009, no-trailing-spaces) --- 106-106: Expected: 0 or 2; Actual: 54 Trailing spaces (MD009, no-trailing-spaces) --- 111-111: Expected: 0 or 2; Actual: 24 Trailing spaces (MD009, no-trailing-spaces) --- 131-131: Expected: 0 or 2; Actual: 1 Trailing spaces (MD009, no-trailing-spaces) --- 160-160: Expected: 0 or 2; Actual: 1 Trailing spaces (MD009, no-trailing-spaces) --- 209-209: Expected: 0 or 2; Actual: 1 Trailing spaces (MD009, no-trailing-spaces) --- 16-16: Expected: 1; Actual: 2 Multiple consecutive blank lines (MD012, no-multiple-blanks) --- 97-97: Expected: 1; Actual: 2 Multiple consecutive blank lines (MD012, no-multiple-blanks) --- 162-162: Expected: 1; Actual: 2 Multiple consecutive blank lines (MD012, no-multiple-blanks) --- 163-163: Expected: 1; Actual: 3 Multiple consecutive blank lines (MD012, no-multiple-blanks) --- 98-98: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 140-140: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 205-205: Expected: 1; Actual: 0; Above Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 209-209: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 204-204: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 68-68: null Spaces inside link text (MD039, no-space-in-links) --- 3-3: null Images should have alternate text (alt text) (MD045, no-alt-text) --- 47-47: null Images should have alternate text (alt text) (MD045, no-alt-text) --- 107-107: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe Table pipe style (MD055, table-pipe-style) </blockquote></details> </blockquote></details> </details> <details> <summary>Additional comments not posted (6)</summary><blockquote> <details> <summary>packages/andromeda-app/src/app.rs (1)</summary><blockquote> `145-157`: **Ensure robust handling of address slicing for cross-chain compatibility.** The changes introduced dynamically adjust the size of the canonical address based on the parent address, which is a critical update for cross-chain compatibility. However, consider adding error handling or validation to ensure that the slicing does not result in an invalid or insecure state. </blockquote></details> <details> <summary>packages/std/src/ado_contract/execute.rs (1)</summary><blockquote> `223-233`: **Clarify and improve AMP packet handling in `execute_amp_receive`.** Renaming the type parameter from `E` to `M` enhances clarity and consistency with Rust naming conventions. The modifications to the AMP packet handling logic are crucial for robust error management. Ensure that the error messages and handling are clear and provide sufficient information for debugging. </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> Line range hint `6-211`: **Comprehensive update to README with detailed information on AndromedaOS.** The additions provide a thorough introduction to AndromedaOS and its components, enhancing the documentation's usefulness for developers and stakeholders. Ensure that all links are functional and that the content remains up-to-date with ongoing project developments. <details> <summary>Tools</summary> <details> <summary>LanguageTool</summary><blockquote> [style] ~23-~23: Consider using the synonym “brief” (= concise, using a few words, not lasting long) to strengthen your wording. Context: ...itecture of the system to develop. A quick description of each of the components t... (QUICK_BRIEF) --- [style] ~28-~28: In American English, abbreviations like “etc.” require a period. Context: ...or referencing ADOs, services, network, etc * [aOS Economics](https://docs.androm... (ETC_PERIOD) --- [uncategorized] ~50-~50: Possible missing comma found. Context: ...ild Applications that span accross many chains taking advantage of all the benefits th... (AI_HYDRA_LEO_MISSING_COMMA) --- [grammar] ~52-~52: Did you mean “too Injective to”? Context: ...leverage some protocol and another part to Injective to leverage some functionality there. The ... (TOO_ADJECTIVE_TO) --- [style] ~60-~60: Consider a shorter alternative to avoid wordiness. Context: ...d need to hire a full development teams in order to build their projects and custom smart ... (IN_ORDER_TO_PREMIUM) --- [style] ~60-~60: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase. Context: ...omeda eliminates this need by providing a very large amount of custom smart contracts that u... (EN_WEAK_ADJECTIVE) --- [uncategorized] ~66-~66: Possible missing comma found. Context: ...s it stands, the ALL contains around 25 ADOs which is the tip of the iceberg. More a... (AI_HYDRA_LEO_MISSING_COMMA) --- [style] ~74-~74: Specify a number, remove phrase, or simply use “many” or “numerous” Context: ...rs access to the following:  1. A large number of production ready ADOs to be used. ... (LARGE_NUMBER_OF) --- [duplication] ~78-~78: Possible typo: you repeated a word Context: ...in that implements the aOS will be able to to see where aOS is also deployed and migh... (ENGLISH_WORD_REPEAT_RULE) --- [uncategorized] ~86-~86: A comma may be missing after the conjunctive/linking adverb ‘Currently’. Context: ...lassified based on their functionality. Currently we have 7 different contract categories... (SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA) --- [grammar] ~91-~91: Consider using either the past participle “allowed” or the present participle “allowing” here. Context: ...ntracts/ecosystem) | Contracts that are allow interaction with different ecosystem pr... (BEEN_PART_AGREEMENT) --- [style] ~105-~105: The preposition ‘amongst’ is correct, but some people think that it is old-fashioned or literary. A more frequently used alternative is the preposition “among”. Context: ...| Contract used to split any sent funds amongst defined addresses. | [Gitbook](https:/... (AMONGST) --- [grammar] ~113-~113: Two determiners in a row. Choose either “an” or “a”. Context: ...ontract that can receive an NFT and run an a sale on it.| [Gitbook](https://docs.and... (DT_DT) </blockquote></details> <details> <summary>Markdownlint</summary><blockquote> 12-12: Expected: h2; Actual: h3 Heading levels should only increment by one level at a time (MD001, heading-increment) --- 84-84: Expected: 0 or 2; Actual: 1 Trailing spaces (MD009, no-trailing-spaces) --- 88-88: Expected: 0 or 2; Actual: 129 Trailing spaces (MD009, no-trailing-spaces) --- 91-91: Expected: 0 or 2; Actual: 22 Trailing spaces (MD009, no-trailing-spaces) --- 92-92: Expected: 0 or 2; Actual: 86 Trailing spaces (MD009, no-trailing-spaces) --- 103-103: Expected: 0 or 2; Actual: 6 Trailing spaces (MD009, no-trailing-spaces) --- 106-106: Expected: 0 or 2; Actual: 54 Trailing spaces (MD009, no-trailing-spaces) --- 111-111: Expected: 0 or 2; Actual: 24 Trailing spaces (MD009, no-trailing-spaces) --- 16-16: Expected: 1; Actual: 2 Multiple consecutive blank lines (MD012, no-multiple-blanks) --- 97-97: Expected: 1; Actual: 2 Multiple consecutive blank lines (MD012, no-multiple-blanks) --- 98-98: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 68-68: null Spaces inside link text (MD039, no-space-in-links) --- 3-3: null Images should have alternate text (alt text) (MD045, no-alt-text) --- 47-47: null Images should have alternate text (alt text) (MD045, no-alt-text) --- 107-107: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe Table pipe style (MD055, table-pipe-style) </blockquote></details> </details> </blockquote></details> <details> <summary>contracts/non-fungible-tokens/andromeda-auction/src/contract.rs (3)</summary><blockquote> `362-379`: **Validation logic for CW20 assets is well-implemented.** The addition of CW20 asset validation ensures that only permitted tokens can be used in auctions. Consider adding detailed comments explaining the conditions under which CW20 assets are deemed valid, to enhance maintainability and clarity for future developers. --- Line range hint `362-390`: **Updated auction logic handles CW20 assets effectively.** The modifications to the auction update logic to include CW20 validation are well thought out and align with the enhancements described in the PR summary. This update ensures that auctions are more robust and can handle different asset types securely. --- `378-379`: **Ensure comprehensive testing of the `validate_denom` function.** Given the critical role of asset validation in auction integrity, it is recommended to ensure that the `validate_denom` function is thoroughly tested, especially with edge cases and different asset types. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
let _res = init( | ||
deps.as_mut(), | ||
Some(AndrAddr::from_string(MOCK_CW20_CONTRACT)), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
Action Required: Update all init
function calls to handle the new optional CW20 address parameter.
The init
function has been updated to include an optional CW20 address parameter. Ensure that all related tests and contract invocations are updated to handle this new parameter correctly. The current implementation has inconsistencies where some invocations still use the old signature.
#!/bin/bash
# Description: Identify invocations of the `init` function that do not correctly handle the new optional CW20 address parameter.
# Test: Search for `init` function usage without the new parameter.
rg --type rust 'init\(deps\.as_mut\(\)(?!,)' --context 2
Analysis chain
Approved: Updated initialization logic in tests to include CW20 address.
The change to include an optional CW20 address in the initialization function is crucial for testing CW20 token integration. Ensure that all related tests and contract invocations are updated to handle this new parameter.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all invocations of the `init` function in test cases handle the new optional CW20 address parameter correctly.
# Test: Search for `init` function usage. Expect: All occurrences should handle the new parameter.
rg --type rust $'init\\('
Length of output: 24531
Motivation
Our development branch ahd gotten out of sync with the
v1.0.x
branch. This PR merges any of the out of sync changes in to our working branch.Notes
A lot of the changes are versioning, a few are added tests that were lost.
Summary by CodeRabbit
Documentation
Bug Fixes
Cargo.toml
files by adding spaces for better readability.andromeda-cw20-staking
contract.New Features
Refactor
ADOContract
module fromE
toM
for clarity.AppComponent
to correctly slice and generate new addresses.