[wip] feat: proposal analyzer skeleton by gustavogama-cll · Pull Request #702 · smartcontractkit/chainlink-deployments-framework

gustavogama-cll · 2026-02-03T15:53:42Z

No description provided.

changeset-bot · 2026-02-03T15:53:57Z

⚠️ No Changeset found

Latest commit: fc1790d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

ecPablo · 2026-02-03T20:56:36Z

engine/cld/mcms/analyzer/internal/engine.go

+)
+
+type analyzerEngine struct {
+	proposalAnalyzers  []analyzer.ProposalAnalyzer


One thing I feel we might need to set on the engine interface is some way to map the operation types to the correct analyzer. Not sure if you have ideas around this. Here's my situation:

As I was working through a concrete example what I found is that I needed a way to map the contract_type+contract_version+function to a concrete analyzer implementation. Otherwise, when we pass in the proposal operations, we'd be trying to apply analyzers to operations that might not be "compatible" with.

Maybe we change this to a map, and let the implementor decide what key to use for mapping this, or we provide some other function to define the mapping

That's a good point. I was thinking we should just let each analyzer handle this, with the assumption that it'll be easy to query this information.

I wonder if it'll scale, though. We might need to experiment a bit.

ecPablo · 2026-02-03T21:09:47Z

engine/cld/mcms/analyzer/types.go

+	BaseAnalyzer
+	// can we merge the contexts? can we replace the ExecutionContext with cldf's Environment?
+	// should the "TimelockProposal be a "DecodeProposal" instead?
+	Analyze(ctx *context.Context, actx AnalyzerContext, ectx ExecutionContext, proposal *mcms.TimelockProposal) (AnalyzedProposal, error)


Context on this comment: I'm trying to implement a specific analyzer that runs on any MCMS.SetConfig contract call and displays a diff of the signers vs the on chain config. Adding some notes as I go through this experience

The first thing I noticed when trying to implement this looking at the signature:

Analyze(ctx *context.Context, actx AnalyzerContext, ectx ExecutionContext, proposal *mcms.TimelockProposal) (AnalyzedProposal, error)

Is that I needed the folllowing:

Some type decoded proposal in go - some easy to access go struct where I can check if proposal.Operation[i] is a SetConfig call. We have the mcm.TimelockProposal but the operations here just gives us the raw bytes.

Putting myself in the shoes of the analyzer dev - right now I'm not clear with this interface if it's my responsibility to build the mappings between the different operation types and the corresponding analyzers or if the engine would be doing this for me. Or if this is just a "global" analysis function.

Ideally we also have a mechanism so that OperationAnalyzer also provisions the decoded operation along with the "raw" mcms operation.

On Another note, I think it would be valuable to document in the design doc the steps a product developer would need to take to implement a concrete analyzer - for example:

Identify the contract type and version to analyze

Identify the function in the contract to analyze

Implement the OperationAnalyzer interface to build a concrete type to analyze the function call.

Register the analyzer in the engine.

Run Engine

Thanks for the comment.

I'm trending torwards your idea that the proposal decoder should indeed be a preliminary step. And that maybe we don't even need the original proposal (assuming the decoded types will contain all relevant metadata)

And I agree the design doc should contain concrete examples -- preferably one for each type of analyzer.

ecPablo · 2026-02-03T21:14:45Z

engine/cld/mcms/analyzer/types.go

+
+type AnalyzerContext interface{}
+
+type ExecutionContext interface{} // domain, environment, etc.


As I was implementing a concrete analyzer, here's the things that I ended up needing:

Blockchains object (ideally pre loaded for me by the engine to the correct domain)

Datastore to access the contracts and contract types.

Decoded proposal object to have an easy to access go struct to distinguish what are the function calls in the proposal and the specific param values, etc.

ecPablo · 2026-02-04T00:40:49Z

I added some comments above, for context, I'm trying to implement a small analyzer for the SetConfig mcms call to add annotations of the new/removed signers. Just listed some of the things that would have helped me implement this. I'll create a PR with the quick & ugly implementation to further discuss but the above comments are the main findings

ChrisAmora · 2026-02-04T20:42:51Z

engine/cld/mcms/analyzer/types.go

+type ProposalAnalyzer interface {
+	BaseAnalyzer
+	Analyze(ctx context.Context, actx AnalyzerContext, ectx ExecutionContext, proposal DecodedTimelockProposal) (Annotations, error)
+}
+
+type BatchOperationAnalyzer interface {
+	BaseAnalyzer
+	Analyze(ctx context.Context, actx AnalyzerContext, ectx ExecutionContext, operation DecodedBatchOperation) (Annotations, error)
+}
+
+type CallAnalyzer interface {
+	BaseAnalyzer
+	Analyze(ctx context.Context, actx AnalyzerContext, ectx ExecutionContext, call DecodedCall) (Annotations, error)
+}
+
+type ParameterAnalyzer interface {
+	BaseAnalyzer
+	Analyze(ctx context.Context, actx AnalyzerContext, ectx ExecutionContext, param DecodedParameter) (Annotations, error)
+}


Looking at rddtool-ccip, seems most analyzers are "CallAnalyzers".
Do you think providing the framework (dependencies, proposal/batch scope, etc.) will encourage teams to write more sophisticated analyzers? Or will most stick with simple validators?

I'm obviously guessing, but I'd say:

75% of the analyzers will be Call analyzers

20% will be Parameter analyzers whose sole purpose is to add annotations to customize rendering

5% will be Proposal analyzers

0% will be BatchOperation analyzers

We do know that there's a demand for Proposal-level analyzers -- it's been mentioned by both CCIP as well as the msig team. But I also think they'll be very complex and as a consequence, much rarer.

if we can find a real world example in rddtool-ccip of a proposal analyzer, that would be great. If not, we can implement something simple, similar (or even simpler) to what the current msig-tools analyzer does.

…ngine

graham-chainlink · 2026-02-10T10:51:32Z

engine/cld/mcms/analyzer/internal/engine.go

+	environmentName string,
+	proposal *mcms.TimelockProposal,
+) (analyzer.AnalyzedProposal, error) {
+	// TODO: instantiate and embed logger in ctx (if not embedded already)


how come we dont pass the logger when constructing the NewAnalyzerEngine and instead use context?

I following the mcms-tools model and passing the logger embedded in the context. See line 105 for an example.

btw, passing a logger in the constructor is a bad pattern, imo. Loggers often carry context, so I much prefer to have them passed along with the context (or as an additional parameter, but that's noisier).

hmm personally i wouldnt say passing logger in the constructor is a bad pattern, it has its benefits like explicitness.
Logger in context is useful when it is request scoped where the logger has been enriced with request specific data.

In this case, i also think it can be a hybrid, the logger gets injected via constructor onto the engine, but pass via context into the different analysers.

hmm personally i wouldnt say passing logger in the constructor is a bad pattern, it has its benefits like explicitness.
Logger in context is useful when it is request scoped where the logger has been enriced with request specific data.

Well, this could turn into a long discussion. Let's just say that I disagree and leave it at that. :-)

Prompt: Enhance the renderer implementation such that: 1. it supports templates defined as files in a pre-defined subfolder. 2. the renderer supports multiple formats. For instance, plain text, markdown, html or even json. The initial implementations should ge plain text and html. 3. annotations should not be rendered as a separate entity. Instead, it should be used to guilde how the renderer behaves. For instance, there could be an annotation that specifies that a given file should be used to render an AnnotatedCall when the format is markdown. Or an annotation may specify that a given AnnotatedParameter is important, and so the template that renders it in the html format should use a "bold" tag, or adds an emoji to highlight the entry. Or the annotation can hold a custom way of rendering the value of the entity, such as an annotation that specifies that a Ethereum address should be rendered in hexadecimal with the "0x" prefix.

ecPablo reviewed Feb 4, 2026

View reviewed changes

ChrisAmora reviewed Feb 4, 2026

View reviewed changes

gustavogama-cll force-pushed the ggama/feat/proposal-analyzer-skeleton branch from e08943b to dc253d6 Compare February 5, 2026 05:24

gustavogama-cll added 4 commits February 10, 2026 01:33

[wip] feat: proposal analyzer skeleton

73830a1

[wip] renamed several types and added a building blocks of analyzer e…

828e221

…ngine

[wip] apply more feedback from review and a few missing parts

480ef85

chore: add Claude Sonnet's initial renderer implementation

3543284

gustavogama-cll force-pushed the ggama/feat/proposal-analyzer-skeleton branch from dc253d6 to 3543284 Compare February 10, 2026 04:33

graham-chainlink reviewed Feb 10, 2026

View reviewed changes

gustavogama-cll force-pushed the ggama/feat/proposal-analyzer-skeleton branch from dd75fd2 to fc1790d Compare February 17, 2026 02:14


		type AnalyzerContext interface{}

		type ExecutionContext interface{} // domain, environment, etc.

Comments

Conversation

gustavogama-cll commented Feb 3, 2026

Uh oh!

changeset-bot bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ecPablo commented Feb 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

changeset-bot bot commented Feb 3, 2026 •

edited

Loading