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.

Sign up for GitHub

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

Jump to bottom

Finish implementation of Validation/ExecutionContext #393

Merged
merged 19 commits into from
Feb 8, 2023
Merged

Finish implementation of Validation/ExecutionContext #393

merged 19 commits into from
Feb 8, 2023

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Feb 1, 2023 •
edited
Loading

Closes: #322

PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests.
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

@codecov
Copy link

codecov bot commented Feb 1, 2023 •
edited
Loading

Codecov Report

Base: 58.45% // Head: 55.69% // Decreases project coverage by -2.77% ⚠️

Coverage data is based on head (9455400) compared to base (4a4fe86).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #393      +/-   ##
==========================================
- Coverage   58.45%   55.69%   -2.77%     
==========================================
  Files         124      122       -2     
  Lines       16259    17065     +806     
==========================================
  Hits         9505     9505              
- Misses       6754     7560     +806
Impacted Files Coverage Δ
crates/ibc/src/applications/transfer/context.rs 35.36% <0.00%> (-6.82%) ⬇️
crates/ibc/src/applications/transfer/relay.rs 0.00% <0.00%> (ø)
...s/ibc/src/clients/ics07_tendermint/client_state.rs 37.89% <0.00%> (-2.66%) ⬇️
crates/ibc/src/core/context.rs 3.79% <0.00%> (-0.95%) ⬇️
crates/ibc/src/core/ics02_client/client_state.rs 75.00% <ø> (ø)
.../src/core/ics04_channel/handler/acknowledgement.rs 43.75% <0.00%> (-36.08%) ⬇️
...ates/ibc/src/core/ics04_channel/handler/timeout.rs 45.16% <0.00%> (-36.60%) ⬇️
...src/core/ics04_channel/handler/timeout_on_close.rs 44.07% <0.00%> (-37.44%) ⬇️
crates/ibc/src/core/ics26_routing/context.rs 43.84% <ø> (ø)
crates/ibc/src/mock/client_state.rs 67.11% <0.00%> (-8.19%) ⬇️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@plafer plafer marked this pull request as ready for review February 1, 2023 23:17
Farhad-Shabani
Farhad-Shabani reviewed Feb 7, 2023
View reviewed changes
Copy link
Member

@Farhad-Shabani Farhad-Shabani left a comment

Choose a reason for hiding this comment

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

Just a few suggestions, otherwise, all set from my end. tnx 👌

crates/ibc/src/clients/ics07_tendermint/client_state.rs Show resolved Hide resolved
crates/ibc/src/clients/ics07_tendermint/client_state.rs Show resolved Hide resolved
crates/ibc/src/mock/context.rs Outdated Show resolved Hide resolved
crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs Outdated Show resolved Hide resolved
@Farhad-Shabani Farhad-Shabani mentioned this pull request Feb 8, 2023
Closed
@plafer plafer mentioned this pull request Feb 8, 2023
Closed
@plafer
Copy link
Contributor Author

plafer commented Feb 8, 2023

@Farhad-Shabani last changes are in (regarding ICS-20 validation for acknowledgement and timeout callbacks):

8915502
e39aaa9

Farhad-Shabani
Farhad-Shabani approved these changes Feb 8, 2023
View reviewed changes
Copy link
Member

@Farhad-Shabani Farhad-Shabani left a comment

Choose a reason for hiding this comment

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

🙏

@Farhad-Shabani Farhad-Shabani merged commit d11e059 into main Feb 8, 2023
@Farhad-Shabani Farhad-Shabani deleted the plafer/322-final branch February 8, 2023 22:26
Farhad-Shabani pushed a commit that referenced this pull request Sep 9, 2024
@plafer
Finish implementation of Validation/ExecutionContext (#393)
87fe1bf 
* timeout validate scaffolding

* validate still not done

* complete timeout validate

* timeout validate: accept noop

* timeout packet execute

* timeout_on_close validation

* timeout_on_close done

* ack validate

* Acknowledgement execute

* fix on_timeout_validate

* ics20 new ack cbs

* ics-20 new timeout cb

* PacketError

* ack: move ordered check

* ics-20 ack packet validate

* ics-20 refund packet validate

* changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Farhad-Shabani Farhad-Shabani Farhad-Shabani approved these changes

Assignees
No one assigned
Labels
None yet
Projects
ibc-rs
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make ValidationContext and ExecutionContext implement the ICS-26 router
2 participants
@plafer @Farhad-Shabani