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

feat(sidecar-extensibility): detect upgradeable contracts and pull ABIs from IPFS #237

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

serichoi65
Copy link

@serichoi65 serichoi65 commented Feb 11, 2025

In order to side-load contracts to Sidecar, we want to add the following features:

  • Detecting an upgradeable contract when we process transaction logs
    • This will not be applicable to EigenLayers' core contracts but only to side-loaded contracts to automatically update contract ABIs
    • This function is not being called in this PR
  • Pulling ABIs from IPFS (main source)
    • The contract metadata needs to exist in IPFS in order for us to pull ABIs. Added 10 seconds timeout during HTTP Get
    • TODO: add a fallback source to pull the data from Etherscan

Non-feature updates:

  • Added AbiFetcher as a new input to ContractManager
  • All tests, except pipelineIntegration_test and restakedStrategies_test, should use a non-paid quiknode url address
  • Updated the ethereum NewClient parameter with http.Client to accept mocked client

References: #223

@serichoi65 serichoi65 requested a review from a team as a code owner February 11, 2025 17:43
@serichoi65 serichoi65 marked this pull request as draft February 11, 2025 17:43
@serichoi65 serichoi65 marked this pull request as ready for review February 13, 2025 18:34
@serichoi65 serichoi65 changed the title detect upgradeable contracts feat(sidecar-extensibility): detect upgradeable contracts Feb 13, 2025
@serichoi65 serichoi65 force-pushed the seri.choi/upgradable-contracts branch 2 times, most recently from c05e33b to d91781e Compare February 13, 2025 19:23
@non-fungible-nelson
Copy link
Collaborator

Hi seri - great PR! Can we have a description please?

@seanmcgary
Copy link
Member

@serichoi65 I'll let you fill in the PR body with implementation specifics.

@non-fungible-nelson I added a link to the related github issue this addresses

@serichoi65 serichoi65 changed the title feat(sidecar-extensibility): detect upgradeable contracts feat(sidecar-extensibility): detect upgradeable contracts and pull ABIs from IPFS Feb 19, 2025
@serichoi65 serichoi65 force-pushed the seri.choi/upgradable-contracts branch from 8c7f61d to dd4f27c Compare February 19, 2025 20:49
// Convert to base58
base58Hash := base58.Encode(bytes)

return fmt.Sprintf("https://ipfs.io/ipfs/%s", base58Hash), nil
Copy link
Member

Choose a reason for hiding this comment

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

This should definitely be taken as a config value that can be overridden at runtime.

)
fmt.Printf("IPFS URL: %s \n", url)

httpClient := &http.Client{
Copy link
Member

Choose a reason for hiding this comment

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

To make things testable, an http client reference should probably be passed to the NewAbiFetcher. That way you could pass in a mocked client.

}
}

func (af *AbiFetcher) FetchMetadataFromAddress(ctx context.Context, address string) (string, string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Naming is a little funky since this returns the ABI not the metadata

Copy link
Author

Choose a reason for hiding this comment

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

It returns bytecodeHash and ABI that I called it metadata, but do you prefer it to be FetchABIFromAddress?

)
fmt.Printf("bytecodeHash: %s", bytecodeHash)

// fetch ABI using IPFS
Copy link
Member

Choose a reason for hiding this comment

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

Since we want to add Etherscan later (or even other options), I would refactor this a little bit into two components:

type AbiFetcher interface {
	GetContractAbiAndMetadata(ctx context.Context, address string) (string, error)
}

type IAbiSource interface {
	FetchAbi(ctx context.Context, address string, bytecode string) (string, error)
}

Move all of the IPFS logic into an implementation of IAbiSource and have the current AbiFetcher take a list of IAbiSources when created. Then all you need to do is iterate over the list in order until you find an ABI or error out.

Copy link
Author

Choose a reason for hiding this comment

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

I'll refactor this in the PR when I add Etherscan

@serichoi65 serichoi65 force-pushed the seri.choi/upgradable-contracts branch from 9e681ad to 9fbc15f Compare February 21, 2025 20:42
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