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

Expand Registry Artifacts to include Deployment Info #133

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

KPrasch
Copy link
Member

@KPrasch KPrasch commented Oct 4, 2023

Includes deployment transaction hash, block number, and deployer account in registry entries.

Example Artifact:

{
    "1337": {
        "TACoApplication": {
            "address": "0x1DAcA792255f6D727Ba28306b4239073C060eCa1",
            "abi": [...],
            "tx_hash": "0x6b5703200e4d1f50290707fb850f72fc066291f87347cbb8f4790684e33525f0",
            "block_number": 5,
            "deployer": "0x2192f6112a026bce4047CeD2A16553Fd31E798B6"
        },
    ...
}

@KPrasch KPrasch marked this pull request as ready for review October 4, 2023 13:37
@derekpierre
Copy link
Member

Haven't looked at the diff as yet, but one thing that stands out based on the description is that merging older registries may be difficult. Remember that we need to merge the registry entry for SubscriptionManager from the old registry files.

Something we need to think about - perhaps #126 also causes this issue... (cc @cygnusv )

@derekpierre
Copy link
Member

Of course we could also have a special case for the older registry since we only need to do it once.

@KPrasch
Copy link
Member Author

KPrasch commented Oct 4, 2023

Of course we could also have a special case for the older registry since we only need to do it once.

#3261 migrates the data format of existing registries appropriately, but in general manual ad-hoc conversion is acceptable for any missing fields.

Copy link
Member

@derekpierre derekpierre left a comment

Choose a reason for hiding this comment

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

🎸

@KPrasch KPrasch merged commit 945e4c4 into nucypher:main Oct 5, 2023
2 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.

3 participants