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

257 FIx Informational Issues - Logic Should be Moved To an Separate Function - Refactoring #388

Merged
merged 6 commits into from
Sep 1, 2023
Merged

257 FIx Informational Issues - Logic Should be Moved To an Separate Function - Refactoring #388

merged 6 commits into from
Sep 1, 2023

Conversation

b-yap
Copy link
Contributor

@b-yap b-yap commented Aug 28, 2023 •
edited
Loading

continuation of #385. Please review the base PR before continuing with this.

  • 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring (partial)

    • from audit:

      • Refactoring is recommended in the function validate_stellar_transaction() from stellar_relay/lib.rs.
      • run_service() from vault/src/system.rs

    • new file

      • validation.rs (covers the logic of validate_stellar_transaction()

@b-yap b-yap changed the title 257 FIx Informational Issues Part 2 257 FIx Informational Issues - Logic Should be Moved To an Separate Function - Refactoring Aug 29, 2023
@b-yap b-yap mentioned this pull request Aug 31, 2023
Closed
26 tasks
@b-yap b-yap force-pushed the 257-fix-informational-issues-2 branch from 3d315b7 to 2f97a8a Compare September 1, 2023 07:34
@b-yap b-yap marked this pull request as ready for review September 1, 2023 07:35
@b-yap b-yap requested a review from a team September 1, 2023 07:36
@b-yap b-yap mentioned this pull request Sep 1, 2023
Merged
ebma
ebma approved these changes Sep 1, 2023
View reviewed changes
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

These are really great changes, thanks for the great refactoring 🎉 I left two nitpicky comments but besides that, IMO it's ready to be merged into the other branch.

pallets/stellar-relay/src/validation.rs Outdated Show resolved Hide resolved
pallets/stellar-relay/src/validation.rs Outdated Show resolved Hide resolved
@b-yap b-yap merged commit 7cec6c2 into 257-fix-informational-issues Sep 1, 2023
@b-yap b-yap deleted the 257-fix-informational-issues-2 branch September 1, 2023 10:01
@b-yap b-yap mentioned this pull request Sep 1, 2023
Merged
b-yap added a commit that referenced this pull request Sep 27, 2023
@b-yap
257 FIx Informational Issues - Logic Should be Moved To an Separate F…
ba47b68 
…unction - Refactoring (#388)

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - partial

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - for System's fn run_service()

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - fix from previous commit

* remove stray log

* remove extra space

* https://github.com/pendulum-chain/spacewalk/pull/388/files#r1312732850,
#388 (comment)
b-yap added a commit that referenced this pull request Oct 2, 2023
@b-yap
257 FIx Informational Issues - Logic Should be Moved To an Separate F…
b431e9c 
…unction - Refactoring (#388)

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - partial

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - for System's fn run_service()

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - fix from previous commit

* remove stray log

* remove extra space

* https://github.com/pendulum-chain/spacewalk/pull/388/files#r1312732850,
#388 (comment)
b-yap added a commit that referenced this pull request Oct 11, 2023
@b-yap
257 fix informational issues part 4 (#393)
7589b69 
* partial of 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring;

partial of STL-03 | Lack of Validation for `destination_address` on `send_payment_to_address()`

* 257 FIx Informational Issues - Logic Should be Moved To an Separate Function - Refactoring (#388)

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - partial

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - for System's fn run_service()

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - fix from previous commit

* remove stray log

* remove extra space

* https://github.com/pendulum-chain/spacewalk/pull/388/files#r1312732850,
#388 (comment)

* 257 Fix informational issues part 3 (#390)

* IML-01 | Same Behavior Defined For Different Conditions

* LI7-05 | Mismatch in Variable Name and Pallet Name

* 9B2-02 | Inconsistent Comments

* PRF-01 | Unhandled Error

* LIH-03 | Values Length Not Validated in `feed_values` Function

* CLI-01 | Confusing Function Naming

* CLI-03 | Incorrect Error Type Thrown partial

* CLI-03 | Incorrect Error Type Thrown

* #390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
https://github.com/pendulum-chain/spacewalk/actions/runs/6048709746/job/16414585252?pr=390#step:12:38

* https://github.com/pendulum-chain/spacewalk/actions/runs/6069946336/job/16465120232?pr=390#step:11:1573

* fix rustfmt error: https://github.com/pendulum-chain/spacewalk/actions/runs/6072497871/job/16472578544?pr=390

* CLI-01 | Confusing Function Naming

* #390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
https://github.com/pendulum-chain/spacewalk/actions/runs/6048709746/job/16414585252?pr=390#step:12:38

* STL-02 | Code Duplication

* SRC-01 | Unused Methods and Storage

* 9B2-05 | Commented Out Code (partial)

* LBC-03 | Inconsistent `match` Expression

* LIY-03 | Reduce Using `unwrap()` and `expect()` in Production Codebase

* TYL-01 | Confusing Variable Naming

* put back the removed dependencies

* https://github.com/pendulum-chain/spacewalk/actions/runs/6095148657/job/16538119439?pr=393#step:11:91

* https://github.com/pendulum-chain/spacewalk/actions/runs/6096401270/job/16541879632#step:11:93

* https://github.com/pendulum-chain/spacewalk/actions/runs/6096401270/job/16541879632#step:11:93

* https://github.com/pendulum-chain/spacewalk/actions/runs/6097368982/job/16544863469?pr=393#step:11:94

* https://github.com/pendulum-chain/spacewalk/actions/runs/6098205449/job/16547533143?pr=393#step:13:19

* #393 (comment),
partly #393 (review)

* #393 (comment)

* #393 (comment),
#393 (review) for vault

* #393 (comment)

* fix https://github.com/pendulum-chain/spacewalk/actions/runs/6263576242/job/17008273208?pr=393#step:12:894

* https://github.com/pendulum-chain/spacewalk/actions/runs/6272775696/job/17034964214?pr=393#step:12:434

* https://github.com/pendulum-chain/spacewalk/actions/runs/6275148901/job/17041998689?pr=393#step:12:587

* https://github.com/pendulum-chain/spacewalk/actions/runs/6276542346/job/17091576766#step:12:476

* #393 (comment)

* #393 (comment) and fix rebase

* 401 fix issues of connecting to stellar overlay network on testnet (#414)

* a 3rd try of #409

* cargo fmt and fix warnings in
https://github.com/pendulum-chain/spacewalk/actions/runs/6459806136/job/17536291785?pr=414

* update the `substrate-stellar-sdk` version

* #414 (comment),
#414 (comment),
#414 (comment),
#414 (comment)

* LIL-01 Dead Code in execute_replace()
LIT-03 Panic can happen between correlated storage modifications

* LIL-01 Dead Code in execute_replace() add back the comment
ebma added a commit that referenced this pull request Oct 12, 2023
@b-yap @ebma
Fix Informational Issues (#385)
8bb0d63 
* SYT-03 | Unnecessary Variable

* STL-03 | Lack of Validation for `destination_address` on `send_payment_to_address()`

* SRL-01 | Usage of Hard-coded Strings

* PAL-03 | Usage of Magic Numbers

* PAL-02 | Unnecessary `Result<...>` Return Type

* CLI-02 | Typos

* 9B2-03 | Unused Errors

* code cleanup for PAL-03 | Usage of Magic Numbers

* code cleanup for PAL-02 | Unnecessary Result<...> Return Type

* partial of 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring;

partial of STL-03 | Lack of Validation for `destination_address` on `send_payment_to_address()`

* code cleanup

* #385 (comment),
#385 (comment),
#385 (comment),
#385 (comment),
#385 (comment),
#385 (comment),
#385 (comment),
#385 (comment)
https://github.com/pendulum-chain/spacewalk/actions/runs/5936258794/job/16098086016?pr=385#step:10:33

* #385 (comment)

* Use vault vs user wallet in integration tests

* fix Unable to get required collateral for amount issue

* #385 (comment)

* removing the "last_known_cursor" and let the CI run it

* https://github.com/pendulum-chain/spacewalk/actions/runs/5972713032/job/16203688884#step:9:144

* Fix `test_replace_succeeds` test case

* fix test case due to changed `HorizonSubmissionError`

* 257 FIx Informational Issues - Logic Should be Moved To an Separate Function - Refactoring (#388)

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - partial

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - for System's fn run_service()

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - fix from previous commit

* remove stray log

* remove extra space

* https://github.com/pendulum-chain/spacewalk/pull/388/files#r1312732850,
#388 (comment)

* 257 Fix informational issues part 3 (#390)

* IML-01 | Same Behavior Defined For Different Conditions

* LI7-05 | Mismatch in Variable Name and Pallet Name

* 9B2-02 | Inconsistent Comments

* PRF-01 | Unhandled Error

* LIH-03 | Values Length Not Validated in `feed_values` Function

* CLI-01 | Confusing Function Naming

* CLI-03 | Incorrect Error Type Thrown partial

* CLI-03 | Incorrect Error Type Thrown

* #390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
https://github.com/pendulum-chain/spacewalk/actions/runs/6048709746/job/16414585252?pr=390#step:12:38

* https://github.com/pendulum-chain/spacewalk/actions/runs/6069946336/job/16465120232?pr=390#step:11:1573

* fix rustfmt error: https://github.com/pendulum-chain/spacewalk/actions/runs/6072497871/job/16472578544?pr=390

* from https://github.com/pendulum-chain/spacewalk/pull/392/files#diff-0f76594421d4493b3bba1c9ed7864cdab7148f19c5d3a18a9bcc9a174bb85aa8

* from https://github.com/pendulum-chain/spacewalk/pull/392/files#diff-0f76594421d4493b3bba1c9ed7864cdab7148f19c5d3a18a9bcc9a174bb85aa8

* 257 fix informational issues part 4 (#393)

* partial of 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring;

partial of STL-03 | Lack of Validation for `destination_address` on `send_payment_to_address()`

* 257 FIx Informational Issues - Logic Should be Moved To an Separate Function - Refactoring (#388)

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - partial

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - for System's fn run_service()

* 9B2-04 | Logic Should be Moved To an Separate Function - Refactoring - fix from previous commit

* remove stray log

* remove extra space

* https://github.com/pendulum-chain/spacewalk/pull/388/files#r1312732850,
#388 (comment)

* 257 Fix informational issues part 3 (#390)

* IML-01 | Same Behavior Defined For Different Conditions

* LI7-05 | Mismatch in Variable Name and Pallet Name

* 9B2-02 | Inconsistent Comments

* PRF-01 | Unhandled Error

* LIH-03 | Values Length Not Validated in `feed_values` Function

* CLI-01 | Confusing Function Naming

* CLI-03 | Incorrect Error Type Thrown partial

* CLI-03 | Incorrect Error Type Thrown

* #390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
https://github.com/pendulum-chain/spacewalk/actions/runs/6048709746/job/16414585252?pr=390#step:12:38

* https://github.com/pendulum-chain/spacewalk/actions/runs/6069946336/job/16465120232?pr=390#step:11:1573

* fix rustfmt error: https://github.com/pendulum-chain/spacewalk/actions/runs/6072497871/job/16472578544?pr=390

* CLI-01 | Confusing Function Naming

* #390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
#390 (comment),
https://github.com/pendulum-chain/spacewalk/actions/runs/6048709746/job/16414585252?pr=390#step:12:38

* STL-02 | Code Duplication

* SRC-01 | Unused Methods and Storage

* 9B2-05 | Commented Out Code (partial)

* LBC-03 | Inconsistent `match` Expression

* LIY-03 | Reduce Using `unwrap()` and `expect()` in Production Codebase

* TYL-01 | Confusing Variable Naming

* put back the removed dependencies

* https://github.com/pendulum-chain/spacewalk/actions/runs/6095148657/job/16538119439?pr=393#step:11:91

* https://github.com/pendulum-chain/spacewalk/actions/runs/6096401270/job/16541879632#step:11:93

* https://github.com/pendulum-chain/spacewalk/actions/runs/6096401270/job/16541879632#step:11:93

* https://github.com/pendulum-chain/spacewalk/actions/runs/6097368982/job/16544863469?pr=393#step:11:94

* https://github.com/pendulum-chain/spacewalk/actions/runs/6098205449/job/16547533143?pr=393#step:13:19

* #393 (comment),
partly #393 (review)

* #393 (comment)

* #393 (comment),
#393 (review) for vault

* #393 (comment)

* fix https://github.com/pendulum-chain/spacewalk/actions/runs/6263576242/job/17008273208?pr=393#step:12:894

* https://github.com/pendulum-chain/spacewalk/actions/runs/6272775696/job/17034964214?pr=393#step:12:434

* https://github.com/pendulum-chain/spacewalk/actions/runs/6275148901/job/17041998689?pr=393#step:12:587

* https://github.com/pendulum-chain/spacewalk/actions/runs/6276542346/job/17091576766#step:12:476

* #393 (comment)

* #393 (comment) and fix rebase

* 401 fix issues of connecting to stellar overlay network on testnet (#414)

* a 3rd try of #409

* cargo fmt and fix warnings in
https://github.com/pendulum-chain/spacewalk/actions/runs/6459806136/job/17536291785?pr=414

* update the `substrate-stellar-sdk` version

* #414 (comment),
#414 (comment),
#414 (comment),
#414 (comment)

* LIL-01 Dead Code in execute_replace()
LIT-03 Panic can happen between correlated storage modifications

* LIL-01 Dead Code in execute_replace() add back the comment

* https://github.com/pendulum-chain/spacewalk/actions/runs/6482197215/job/17610217546?pr=385#step:12:560

* https://github.com/pendulum-chain/spacewalk/actions/runs/6491793772/job/17629635299?pr=385#step:11:58

* fix clippy issues

* cargo fmt --all

---------

Co-authored-by: Marcel Ebert <mail@marcel-ebert.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebma ebma ebma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@b-yap @ebma