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

Adds prototype NFT Ownership flow for use in demo #44

Merged
merged 1 commit into from Feb 16, 2023

Conversation

krhoda
Copy link
Contributor

@krhoda krhoda commented Feb 14, 2023

Description

Adds a prototype NFT Ownership Verification flow. The implementation will likely change subsequently to it's use in a demo.

Notes on the nature of the changes to come are found in the comments and documentation. Until a final version of the NFT Ownership flow lands, Gitbook has been skipped. The documentation primarily says don't rely on it.

Type

  • New feature (non-breaking change which adds functionality)

Diligence Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas.

@krhoda
Copy link
Contributor Author

krhoda commented Feb 14, 2023

Need to finish adding to the docs and squashing commits before removing WIP.

@cloudflare-pages
Copy link

cloudflare-pages bot commented Feb 14, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 22545d2
Status: ✅  Deploy successful!
Preview URL: https://5667445c.rebase.pages.dev
Branch Preview URL: https://feat-alchemy-nft-gate.rebase.pages.dev

View logs

@krhoda
Copy link
Contributor Author

krhoda commented Feb 15, 2023

WIP Closed, ready for review.

@krhoda krhoda changed the title [WIP] Adds prototype NFT Ownership flow for use in demo Adds prototype NFT Ownership flow for use in demo Feb 15, 2023
rust/rebase/src/proof/nft_ownership.rs Show resolved Hide resolved
rust/rebase/src/flow/nft_ownership.rs Outdated Show resolved Hide resolved

let client = Client::new();
// NOTE: Intentionally re-assigning before read, thus using: #[allow(unused_assignments)]
#[allow(unused_assignments)]
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to modify them in the loop. I re-assign based on the first iteration.

Copy link
Member

Choose a reason for hiding this comment

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

page_key is assigned and then directly used, so you don't need it to be "global", you can just do

let page_key = res.next_page.clone();

Copy link
Contributor Author

@krhoda krhoda Feb 15, 2023

Choose a reason for hiding this comment

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

res is used after the loop, how do I do that? I tried returning from the loop, but that seems to return out of the function:

     let res = loop {
            let inner_res = self
                .process_page(&client, next_u.clone(), &proof.statement.contract_address)
                .await?;

            if inner_res.found {
                return inner_res;
            }

            let page_key = inner_res.next_page.clone();
            match page_key {
                None => return inner_res,
                Some(s) => {
                    next_u = u.join(&format!("&pageKey={}", s)).map_err(|_e| {
                        FlowError::BadLookup("Could not follow paginated results".to_string())
                    })?;
                }
            }
        };

gets:

warning: unreachable statement
 --> src/flow/nft_ownership.rs:190:9
  |
170 |           let res = loop {
  |  ___________________-
171 | |             let inner_res = self
172 | |                 .process_page(&client, next_u.clone(), &proof.statement.contract_address)
173 | |                 .await?;
...   |
187 | |             }
188 | |         };
  | |_________- any code following this expression is unreachable

And:

error[E0308]: mismatched types
  --> src/flow/nft_ownership.rs:176:24
   |
176 |                 return inner_res;

Copy link
Member

Choose a reason for hiding this comment

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

You can do break inner_res

#[allow(unused_assignments)]
let mut page_key: Option<String> = None;
#[allow(unused_assignments)]
let mut res: PageResult = PageResult {
Copy link
Member

Choose a reason for hiding this comment

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

Don't use a mut, return from the loop

Copy link
Contributor Author

@krhoda krhoda Feb 15, 2023

Choose a reason for hiding this comment

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

How do I modify these values between iterations of the loop?

Copy link
Member

@sbihel sbihel Feb 15, 2023

Choose a reason for hiding this comment

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

Like page_key, res is assigned and used in the same loop, so I would do

     let res = loop {
            let res = self
                .process_page(&client, next_u.clone(), &proof.statement.contract_address)
                .await?;

            if res.found {
                break;
            }

            page_key = res.next_page.clone();
            match page_key {
                None => break res,
                Some(s) => {
                    next_u = u.join(&format!("&pageKey={}", s)).map_err(|_e| {
                        FlowError::BadLookup("Could not follow paginated results".to_string())
                    })?;
                }
            }
        };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about next_u?

Copy link
Member

Choose a reason for hiding this comment

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

In that case it's fine because it's changed in one iteration of the loop and used in the next one. But actually, is this correct? Because it just keeps adding &pageKey={} to the URL?

Copy link
Contributor Author

@krhoda krhoda Feb 15, 2023

Choose a reason for hiding this comment

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

If you look close, I keep using the url that next_u is cloned from. I never modify the original url, just use it to generate next_u.

Copy link
Member

Choose a reason for hiding this comment

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

indeed! 😪

…ation. Lacks full documentation or appearance in demo dapp because breaking changes are expected. Is implemented in NPM lib and worker for use in seperate demo.t
@krhoda krhoda merged commit 941460c into main Feb 16, 2023
@krhoda krhoda deleted the feat/alchemy-nft-gate branch June 14, 2023 15:36
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.

None yet

2 participants