fix: inbound index

[kingpinXD]

Description

• Adds an index to keep track of finalized inbounds
• Adds a new field to the CCTX struct to track the finalization state of a tx

Closes: (https://github.com/zeta-chain/security/issues/44)

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

• Tested CCTX in localnet
• Tested in development environment
• Go unit tests
• Go integration tests
• Tested via GitHub Actions

Checklist:

• I have added unit tests that prove my fix feature works

[lumtis]

It looks good to me.
I'm thinking in anticipation of #1515 if we could consider instead of having a boolean IsFinalized introduce a inboundStatus
So we can have:

• nonFinalized: vote still ongoing
• finalizing: vote finished but not executed yet
• finalized: voted and executed

We could introduce the status value and just keep nonFinalized and finalized for now

[kingpinXD]

It looks good to me. I'm thinking in anticipation of #1515 if we could consider instead of having a boolean IsFinalized introduce a inboundStatus So we can have:

• nonFinalized: vote still ongoing
• finalizing: vote finished but not executed yet
• finalized: voted and executed

We could introduce the status value and just keep nonFinalized and finalized for now

It makes sense to do that,however we can consider the following situations

• if 1515 is going to be part of v12 or the first mainnet release , it would be cleaner to add that as part of the implementation PR
• if 1515 is going to be implemented post mainnet, we can modify the cross-chain struct now and avoid migrating cctxs on mainnet

[github-actions]

!!!WARNING!!!
nosec detected in the following files: x/crosschain/client/integrationtests/cli_helpers.go, x/crosschain/keeper/msg_server_vote_inbound_tx.go, x/crosschain/keeper/msg_server_vote_outbound_tx.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

[brewmaster012]

zeta-chain/security#44

What's the relationship to this issue?

[kingpinXD]

zeta-chain/security#44

What's the relationship to this issue?

it adds a check for the inbound which have been finalized
https://github.com/zeta-chain/zeta-node/blob/0a41cccbd0ae2c72afe350fe4b7ac060f1d9a1eb/x/crosschain/keeper/msg_server_vote_inbound_tx.go#L108-L111

[brewmaster012]

zeta-chain/security#44

What's the relationship to this issue?

it adds a check for the inbound which have been finalized https://github.com/zeta-chain/zeta-node/blob/0a41cccbd0ae2c72afe350fe4b7ac060f1d9a1eb/x/crosschain/keeper/msg_server_vote_inbound_tx.go#L108-L111

so it's basically using the txhash+chainid+eventid as a stable "index" for deduplicating observation?

[lumtis]

To me it seems a correct way to handle it. We can highlight the fix with TrailOfBits in our next call.