fix(logic)!: remove unsafe prolog predicates from registry #710
Conversation
WalkthroughThe recent changes in the codebase involve the removal of two Prolog predicates, Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## main #710 +/- ##
=======================================
Coverage 54.42% 54.42%
=======================================
Files 73 73
Lines 2901 2901
=======================================
Hits 1579 1579
Misses 1228 1228
Partials 94 94 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- x/logic/interpreter/registry.go (1 hunks)
Additional comments not posted (3)
x/logic/interpreter/registry.go (3)
Line range hint
89-139:
Verify theRegisterfunction.Ensure that the removal of
set_prolog_flag/2andhalt/1does not affect the functionality of theRegisterfunction.Verification successful
Verification successful.
The removal of
set_prolog_flag/2andhalt/1does not affect the functionality of theRegisterfunction as these predicates are not used within it.
set_prolog_flag/2was not found in the codebase.halt/1was only found in test files, indicating it does not impact theRegisterfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the removal of `set_prolog_flag/2` and `halt/1` does not affect the functionality of the `Register` function. # Test: Search for `set_prolog_flag/2` in the `Register` function. Expect: No occurrences. rg --type python -A 5 $'set_prolog_flag/2' # Test: Search for `halt/1` in the `Register` function. Expect: No occurrences. rg --type python -A 5 $'halt/1'Length of output: 134
Script:
#!/bin/bash # Description: Verify that the removal of `set_prolog_flag/2` and `halt/1` does not affect the functionality of the `Register` function. # Test: Search for `set_prolog_flag/2` in the Go files. Expect: No occurrences. rg --type go -A 5 $'set_prolog_flag/2' # Test: Search for `halt/1` in the Go files. Expect: No occurrences. rg --type go -A 5 $'halt/1'Length of output: 1021
Line range hint
80-85:
Verify theRegistryNamesvariable.Ensure that the removal of
set_prolog_flag/2andhalt/1is reflected in theRegistryNamesvariable.Verification successful
Verification successful:
set_prolog_flag/2andhalt/1are not present in theRegistryNamesvariable.The predicates
set_prolog_flag/2andhalt/1have been successfully removed from theRegistryNamesvariable in thex/logic/interpreter/registry.gofile.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `set_prolog_flag/2` and `halt/1` are not present in the `RegistryNames` variable. # Test: Search for `set_prolog_flag/2` in the `RegistryNames` variable. Expect: No occurrences. rg --type python -A 5 $'set_prolog_flag/2' # Test: Search for `halt/1` in the `RegistryNames` variable. Expect: No occurrences. rg --type python -A 5 $'halt/1'Length of output: 134
Script:
#!/bin/bash # Description: Verify that `set_prolog_flag/2` and `halt/1` are not present in the `RegistryNames` variable. # Test: Search for `set_prolog_flag/2` in the `RegistryNames` variable. Expect: No occurrences. rg --type go -A 5 $'set_prolog_flag/2' # Test: Search for `halt/1` in the `RegistryNames` variable. Expect: No occurrences. rg --type go -A 5 $'halt/1'Length of output: 1021
Script:
#!/bin/bash # Description: Verify that `set_prolog_flag/2` and `halt/1` are not present in the `RegistryNames` variable in the registry.go file. # Test: Search for `set_prolog_flag/2` in the `RegistryNames` variable within the registry.go file. Expect: No occurrences. rg --type go -A 5 $'set_prolog_flag/2' x/logic/interpreter/registry.go # Test: Search for `halt/1` in the `RegistryNames` variable within the registry.go file. Expect: No occurrences. rg --type go -A 5 $'halt/1' x/logic/interpreter/registry.goLength of output: 128
Line range hint
13-13:
Verify the removal ofset_prolog_flag/2andhalt/1.Ensure that the removal of these predicates aligns with the objectives of the PR and does not introduce any issues. Verify that these predicates are not used elsewhere in the codebase.
Closes #624
Closes #617
Closes #336
Purpose
This PR aims to make the logic module prolog vm finally safe, by removing unsafe predicates from the registry. By doing so they cannot be added even through a governance proposal.
The idea is to expose through the registry only safe to use predicates.
Details
Since the recent changes in the VM I was honestly quite disappointed to discover only 2 unsafe predicates 😅. AFAIS previously suspected predicates have all been fixed by the improved VM.
Here are the removed predicates:
halt/1: Make a node to halt, if used in the context of a transaction the network is stopped;set_prolog_flag/2: This one doesn't seem really unsafe to me regarding the flags we are able to change, but the intent of changing the VM's behaviour (e.g. enable debug mode) is not desirable to me;Let me know if you have any other potentially unsafe predicates in mind.
Summary by CodeRabbit
New Features
Bug Fixes