Skip to content

CCIP-4796 ccip analyze mcms proposal - #16384

Merged
bukata-sa merged 5 commits into
developfrom
feature/analyze_proposal_v2
Feb 25, 2025
Merged

CCIP-4796 ccip analyze mcms proposal#16384
bukata-sa merged 5 commits into
developfrom
feature/analyze_proposal_v2

Conversation

@bukata-sa

@bukata-sa bukata-sa commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

This PR is needed to be merged before https://github.com/smartcontractkit/chainlink-deployments/pull/695

Introducing TxCallAnalyzer which takes raw tx data, abi, methodId and produces decoded data that is ready to be formatted into human readable output.
Working with Arguments to receive human readable output is made in chainlink-deployments here https://github.com/smartcontractkit/chainlink-deployments/pull/695

@Osazperry Osazperry left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Update All Systems

@bukata-sa
bukata-sa force-pushed the feature/analyze_proposal_v2 branch 2 times, most recently from 63384ba to 66331b8 Compare February 13, 2025 14:59
@github-actions

github-actions Bot commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@bukata-sa
bukata-sa force-pushed the feature/analyze_proposal_v2 branch 13 times, most recently from ae5a0b4 to 9e74f6a Compare February 17, 2025 16:50
@bukata-sa
bukata-sa force-pushed the feature/analyze_proposal_v2 branch 7 times, most recently from 27fd8a9 to 28a6914 Compare February 20, 2025 09:41
Comment thread deployment/ccip/changeset/deployer_group.go Outdated
Comment thread deployment/ccip/changeset/deployer_group.go Outdated
Comment thread deployment/common/proposalutils/analyze.go Outdated
Comment thread deployment/common/proposalutils/analyze.go Outdated
@bukata-sa
bukata-sa force-pushed the feature/analyze_proposal_v2 branch from 16f7f67 to f9d1aaa Compare February 21, 2025 08:47
kylesmartin
kylesmartin previously approved these changes Feb 21, 2025
@bukata-sa
bukata-sa force-pushed the feature/analyze_proposal_v2 branch from f9d1aaa to f3d6c35 Compare February 24, 2025 10:45
@bukata-sa
bukata-sa enabled auto-merge February 24, 2025 13:12

@AnieeG AnieeG left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you please add a test for deployer_group as well?

Comment thread deployment/ccip/changeset/state.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +196 to +211
if abiStr, ok := d.state.Chains[chain].ABIByAddress[tx.To().Hex()]; ok {
_abi, err := abi.JSON(strings.NewReader(abiStr))
if err != nil {
d.e.Logger.Errorw("could not load ABI",
"chain", chain, "address", tx.To().Hex(), "error", err)
} else {
decodedCall, err := d.txDecoder.Analyze(tx.To().String(), &_abi, tx.Data())
if err != nil {
d.e.Logger.Errorw("could not analyze transaction",
"chain", chain, "address", tx.To().Hex(), "nonce", currentNonce, "error", err)
} else {
description = decodedCall.Describe(d.describeContext)
}
}
}
dc.transactions[chain] = append(dc.transactions[chain], DescribedTransaction{Tx: tx, Description: description})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you just want to log the err and not return?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed: return error if couldn't load ABI
but log error and continue if error during analyze. That's because I want to keep it decoding optional if something goes wrong here

@bukata-sa
bukata-sa force-pushed the feature/analyze_proposal_v2 branch from 349ec20 to d90aadb Compare February 25, 2025 15:31
@cl-sonarqube-production

Copy link
Copy Markdown

@bukata-sa
bukata-sa added this pull request to the merge queue Feb 25, 2025
Merged via the queue into develop with commit 9abe695 Feb 25, 2025
@bukata-sa
bukata-sa deleted the feature/analyze_proposal_v2 branch February 25, 2025 17:27
asoliman92 pushed a commit that referenced this pull request Feb 26, 2025
* CCIP-4796 ccip analyze mcms proposal

* mcms analyze

* integrate into deployer group

* review fixes

* review fixes
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.

4 participants