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

refactor: cctx validate inbound #2340

Merged
merged 51 commits into from
Jun 21, 2024
Merged

refactor: cctx validate inbound #2340

merged 51 commits into from
Jun 21, 2024

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Jun 12, 2024

Description

NOTE: do not merge since it needs one more review and to make review of base branch PR easier.

Left some open points for discussion in comments.

Closes: #2277

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

Copy link

codecov bot commented Jun 13, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 68.73%. Comparing base (ca9b90f) to head (7f3bf86).

Current head 7f3bf86 differs from pull request most recent head e835137

Please upload reports for the commit e835137 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2340      +/-   ##
===========================================
+ Coverage    68.66%   68.73%   +0.06%     
===========================================
  Files          302      304       +2     
  Lines        19153    19169      +16     
===========================================
+ Hits         13152    13176      +24     
+ Misses        5351     5347       -4     
+ Partials       650      646       -4     
Files Coverage Δ
app/app.go 2.18% <ø> (+0.01%) ⬆️
x/crosschain/keeper/cctx_gateway_observers.go 100.00% <100.00%> (ø)
x/crosschain/keeper/cctx_gateway_zevm.go 100.00% <100.00%> (ø)
x/crosschain/keeper/cctx_gateways.go 100.00% <100.00%> (ø)
...hain/keeper/cctx_orchestrator_validate_outbound.go 94.92% <100.00%> (ø)
x/crosschain/keeper/cctx_utils.go 100.00% <100.00%> (ø)
x/crosschain/keeper/evm_deposit.go 91.25% <100.00%> (ø)
.../crosschain/keeper/msg_server_migrate_tss_funds.go 83.44% <100.00%> (+0.22%) ⬆️
x/crosschain/keeper/msg_server_vote_inbound_tx.go 100.00% <100.00%> (+8.00%) ⬆️
x/crosschain/keeper/msg_server_whitelist_erc20.go 80.15% <100.00%> (+0.30%) ⬆️
... and 4 more

... and 34 files with indirect coverage changes

x/crosschain/keeper/cctx_gateway_observers.go Show resolved Hide resolved
x/crosschain/keeper/cctx_gateway_observers.go Show resolved Hide resolved
x/crosschain/keeper/cctx_gateway_observers.go Outdated Show resolved Hide resolved
x/crosschain/keeper/cctx_gateway_observers.go Outdated Show resolved Hide resolved
x/crosschain/keeper/cctx_gateway_observers.go Outdated Show resolved Hide resolved
x/crosschain/keeper/initiate_outbound.go Outdated Show resolved Hide resolved
x/crosschain/keeper/initiate_outbound.go Outdated Show resolved Hide resolved
x/crosschain/keeper/cctx_orchestrator_validate_inbound.go Outdated Show resolved Hide resolved
x/crosschain/keeper/msg_server_vote_inbound_tx.go Outdated Show resolved Hide resolved
x/crosschain/keeper/evm_hooks.go Outdated Show resolved Hide resolved
@lumtis
Copy link
Member

lumtis commented Jun 17, 2024

I think we can rename this PR as refactor instead of feat.
Same for #2317

@skosito skosito changed the title feat: cctx validate inbound refactor: cctx validate inbound Jun 17, 2024
@skosito skosito requested a review from lumtis June 18, 2024 13:41
@skosito skosito linked an issue Jun 18, 2024 that may be closed by this pull request
Base automatically changed from cctx-validate-outbound to develop June 19, 2024 18:25
@skosito skosito requested a review from swift1337 as a code owner June 19, 2024 18:25
Copy link

gitguardian bot commented Jun 19, 2024

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
11759679 Triggered Generic High Entropy Secret 86c1f5c cmd/zetae2e/local/accounts.go View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@skosito skosito merged commit e6287e2 into develop Jun 21, 2024
16 of 17 checks passed
@skosito skosito deleted the cctx-validate-inbound branch June 21, 2024 02:30
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.

Implement a ValidateInbound method in crosschain keeper
3 participants