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

Removing remaining clippy errors from Bubblegum tests #1097

Merged
merged 1 commit into from
May 19, 2023

Conversation

danenbm
Copy link
Contributor

@danenbm danenbm commented May 15, 2023

  • Changes the Result types in tests/utils/mod.rs to use Box<Error> for the error variant, to fix the clippy issue of large enum variant return.
  • Removes use of RefCell for Tree and TxBuilder objects. It looks like this was done to allow immutable references to be used for most method calls, and allow for immutable and mutable references to be used at the same time.
    • Overall the use of RefCell was convenient but I don't think it is the best fit for this use case. I'd rather just follow the borrow checker rules. Also, it is not correct to hold a RefCell over await statements, which is fairly fundamental to the test architecture used here.
    • For example, before this code change, a method such as zero_leaf would take an immutable reference to a Tree object instance, and then mutate it because it had a RefCell so it could do a borrow_mut():
    pub fn zero_leaf(&self, index: u32) -> Result<()> {
        let node = [0u8; 32];
        // The conversion below should never fail.
        self.proof_tree
            .borrow_mut()
            .add_leaf(node, usize::try_from(index).unwrap());
        Ok(())
    }
    • Also this allowed taking an immutable reference to a keypair and sending it to a method that would normally require a mutable reference. Thus the borrow checker would not like this either.

@danenbm danenbm changed the base branch from master to danenbm/verified-col-bgum May 15, 2023 23:57
@danenbm danenbm marked this pull request as ready for review May 16, 2023 16:47
@danenbm danenbm requested a review from a team as a code owner May 16, 2023 16:48
@danenbm danenbm requested review from samuelvanderwaal and lorisleiva and removed request for a team May 16, 2023 16:48
Base automatically changed from danenbm/verified-col-bgum to master May 17, 2023 16:52
@danenbm danenbm requested a review from febo May 17, 2023 16:59
@danenbm danenbm merged commit 308bae6 into master May 19, 2023
@danenbm danenbm deleted the danenbm/bgum-test-clippy branch May 19, 2023 16:01
blockiosaurus added a commit that referenced this pull request Jun 1, 2023
* chore: Release

* Update generated SDK and IDL versions for token-metadata 1.11.0 release

* Bubblegum collection docs fix (#1072)

* collection fix

* Revert "collection fix"

This reverts commit cbc20ca.

* change docs for bubblegum

* Allow for 5 creators in Bubblegum (#1086)

* Update token metadata dependency

* Allow data and is_immutable to be updated at same time (#1090)

* Added can_update_data_and_is_mutable_same_instruction to test.
* Also added a couple more tests to Update for other cases.

* Remove Old Testing CLI (#1089)

* remove old CLI

* add README to replace CLI

* chore: Release

* Update generated SDK and IDL versions for token-metadata 1.11.1 release

* Add missing delegate record error (#1093)

* Adding print instruction.

* Add delegate check on transfer (#1095)

* Test delegate status after transfer

* Add delegate check on transfer

* add migration role exception (#1096)

* Allow Bubblegum to create metadata with verified collection (#1087)

* Allow Bubblegum to create metadata with verified collection

* Use separate variable

* Fix ordering in collection verification

* Add test for verifying collection and decompressing
with verified collection.
* Add DigitalAsset from token-metadata in test utils
for creating a collection parent.
* Add needed helper objects such as VerifyCollectionBuilder,
as well as required local test tree updates.
* Fix some clippy issues in the contract.  There are still a
few more clippy issues in the tests but they are more involved.

* Cleanup comment

* Updating based on feedback.

* detect deprecated ixes and provide proper error (#1092)

* detect deprecated ixes and provide proper error

* run cargo fmt

* Removing comments.

* Adding UA check.

* Making code rustier and adding ME check.

* Box test utils err and remove RefCells from test objects (#1097)

* More minor simplifications to Bubblegum BPF tests (#1101)

* Making UA check work for any standard.

* Adding init check.

---------

Co-authored-by: Michael Danenberg <[email protected]>
Co-authored-by: ethyi <[email protected]>
Co-authored-by: Fernando Otero <[email protected]>
Co-authored-by: Samuel Vanderwaal <[email protected]>
blockiosaurus added a commit that referenced this pull request Jul 14, 2023
* Adding edition_marker_v2.

* Adding marker seed.

* Adding divisor to prevent overalloc.

* Responding to feedback and fixing bug.

* Adding Print Instruction (#1094)

* chore: Release

* Update generated SDK and IDL versions for token-metadata 1.11.0 release

* Bubblegum collection docs fix (#1072)

* collection fix

* Revert "collection fix"

This reverts commit cbc20ca.

* change docs for bubblegum

* Allow for 5 creators in Bubblegum (#1086)

* Update token metadata dependency

* Allow data and is_immutable to be updated at same time (#1090)

* Added can_update_data_and_is_mutable_same_instruction to test.
* Also added a couple more tests to Update for other cases.

* Remove Old Testing CLI (#1089)

* remove old CLI

* add README to replace CLI

* chore: Release

* Update generated SDK and IDL versions for token-metadata 1.11.1 release

* Add missing delegate record error (#1093)

* Adding print instruction.

* Add delegate check on transfer (#1095)

* Test delegate status after transfer

* Add delegate check on transfer

* add migration role exception (#1096)

* Allow Bubblegum to create metadata with verified collection (#1087)

* Allow Bubblegum to create metadata with verified collection

* Use separate variable

* Fix ordering in collection verification

* Add test for verifying collection and decompressing
with verified collection.
* Add DigitalAsset from token-metadata in test utils
for creating a collection parent.
* Add needed helper objects such as VerifyCollectionBuilder,
as well as required local test tree updates.
* Fix some clippy issues in the contract.  There are still a
few more clippy issues in the tests but they are more involved.

* Cleanup comment

* Updating based on feedback.

* detect deprecated ixes and provide proper error (#1092)

* detect deprecated ixes and provide proper error

* run cargo fmt

* Removing comments.

* Adding UA check.

* Making code rustier and adding ME check.

* Box test utils err and remove RefCells from test objects (#1097)

* More minor simplifications to Bubblegum BPF tests (#1101)

* Making UA check work for any standard.

* Adding init check.

---------

Co-authored-by: Michael Danenberg <[email protected]>
Co-authored-by: ethyi <[email protected]>
Co-authored-by: Fernando Otero <[email protected]>
Co-authored-by: Samuel Vanderwaal <[email protected]>

* Add the ability to burn a ProgrammableNonfungibleEdition (#1108)

* Adding print instruction.

* Updating based on feedback.

* Removing comments.

* Adding UA check.

* Making code rustier and adding ME check.

* Making UA check work for any standard.

* Adding init check.

* Adding burn PR for pNFTEs

* Fixing burn byte offset.

* Adding escrows for pNFTs and removing check for closing the account. (#1110)

* Manually reducing size of Bubblegum IDL for publishing

* Setting Solana to exact version in Bubblegum

* Add operation list (#1098)

* Add operation list

* Update packages version

* Fix typo

* Add operation definition

* Typos

* chore: Release mpl-bubblegum version 0.8.0

* Update Bubblegum npm package version prior to publish

* Fixing in response to feedback.

* replace serialize with metadata save fn

* fix overwriting of fee flag on 'update' (#1114)

* fix overwriting of fee flag on 'update'

* fix index in clean write

---------

Co-authored-by: Michael Danenberg <[email protected]>
Co-authored-by: Fernando Otero <[email protected]>
Co-authored-by: Samuel Vanderwaal <[email protected]>

* Adding fees and fixing merge errors. (#1116)

* Adding fees and fixing merge errors.

* Adding token standard flag.

* Adding delegate for pNFT editions. (#1130)

* Adding fees for standard NFT editions. (#1131)

* Adding supply and decimal checks. (#1132)

* Adding mint checks. (#1140)

* Removing tests built for statically sized accounts.

* Generating the JS SDK.

* Fixing burn tests and modifying the Edition close auth for utility delegate (#1144)

* Fixing burn test issues.

* Moving UA check up a level.

* Fixing an old test.

* Make mint account optional signer (#1146)

* Fixing burn test issues.

* Moving UA check up a level.

* Fixing an old test.

* Making the mint account an optional signer.

* Removing commented out code.

* Removing unnecessary error.

* Regen JS

---------

Co-authored-by: Michael Danenberg <[email protected]>
Co-authored-by: ethyi <[email protected]>
Co-authored-by: Fernando Otero <[email protected]>
Co-authored-by: Samuel Vanderwaal <[email protected]>
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