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

🐛 Fix breaking issue with revoked passports #6

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

acemasterjb
Copy link

Fixes #5 .

Changes proposed in this pull request:

  • 🐛 Fix issue with revoked passports, use PassportIssuer subgraph to filter out passports which are revoked
    • utilises subgraph to get list of revoked ids

All changes pass the following tests:

yarn test --strategy=nation3-passport-coop-with-delegations

github-actions bot and others added 2 commits January 4, 2024 09:00
Co-authored-by: ChaituVR <ChaituVR@users.noreply.github.com>
Copy link

@johnmark13 johnmark13 left a comment

Choose a reason for hiding this comment

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

I love this work, and so fast, evn if I don't think you should move constants around inside another PR just to have them in a neat order. I am only holding off approval because of other comments on the subgraph PR.

blockTag
});

const passportIssuanceSubgrgraph = "https://api.thegraph.com/subgraphs/name/nation3/passportissuance";

Choose a reason for hiding this comment

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

I think that this should be in configuration just because in theory this is a strategy that could be used by other projects, even if that is really unlikely! maybe just the nation3/passsportissuance part...

blockTag
});

const passportIssuanceSubgrgraph = "https://api.thegraph.com/subgraphs/name/nation3/passportissuance";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const passportIssuanceSubgrgraph = "https://api.thegraph.com/subgraphs/name/nation3/passportissuance";
const passportIssuanceSubgraph = "https://api.thegraph.com/subgraphs/name/nation3/passportissuance";

@johnmark13 johnmark13 merged commit d7d98b7 into nation3:master Jan 25, 2024
5 checks passed
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.

Add handling of revoked passports
3 participants