CCIP-4796 ccip analyze mcms proposal - #16384
Merged
Merged
Conversation
bukata-sa
requested review from
0xAustinWang,
0xnogo,
asoliman92,
dimkouv,
makramkd,
mateusz-sekara and
winder
February 13, 2025 14:47
bukata-sa
force-pushed
the
feature/analyze_proposal_v2
branch
from
February 13, 2025 14:51
0d7cc94 to
6c4d95f
Compare
bukata-sa
force-pushed
the
feature/analyze_proposal_v2
branch
2 times, most recently
from
February 13, 2025 14:59
63384ba to
66331b8
Compare
Contributor
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
bukata-sa
force-pushed
the
feature/analyze_proposal_v2
branch
13 times, most recently
from
February 17, 2025 16:50
ae5a0b4 to
9e74f6a
Compare
bukata-sa
force-pushed
the
feature/analyze_proposal_v2
branch
7 times, most recently
from
February 20, 2025 09:41
27fd8a9 to
28a6914
Compare
kylesmartin
reviewed
Feb 20, 2025
bukata-sa
force-pushed
the
feature/analyze_proposal_v2
branch
from
February 21, 2025 08:47
16f7f67 to
f9d1aaa
Compare
kylesmartin
previously approved these changes
Feb 21, 2025
bukata-sa
force-pushed
the
feature/analyze_proposal_v2
branch
from
February 24, 2025 10:45
f9d1aaa to
f3d6c35
Compare
bukata-sa
enabled auto-merge
February 24, 2025 13:12
AnieeG
reviewed
Feb 24, 2025
AnieeG
left a comment
Contributor
There was a problem hiding this comment.
can you please add a test for deployer_group as well?
Contributor
There was a problem hiding this comment.
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}) |
Contributor
There was a problem hiding this comment.
Do you just want to log the err and not return?
Contributor
Author
There was a problem hiding this comment.
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
force-pushed
the
feature/analyze_proposal_v2
branch
from
February 25, 2025 11:49
ad45142 to
349ec20
Compare
bukata-sa
force-pushed
the
feature/analyze_proposal_v2
branch
from
February 25, 2025 15:31
349ec20 to
d90aadb
Compare
|
AnieeG
approved these changes
Feb 25, 2025
kylesmartin
approved these changes
Feb 25, 2025
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




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