giteadmin/SoLUSH-3
3
0
Fork 0
Code Issues 11 Pull Requests 3 Packages Projects Releases Wiki Activity

Boolean stack implementation #18

Open
navelbuffle wants to merge 17 commits from boolean-stack into offchain-start
pull from: boolean-stack
merge into: :offchain-start
Conversation 1 Commits 17 Files Changed 5
+912 -170
navelbuffle commented 2025-01-05 06:25:01 +00:00
Collaborator
Copy Link
Copy Source

The goal of this PR is to implement bool stack and cover with unit tests to solve:

I consider all issue are solved by this PR with minor tech dept.

What is not solved:

  • opcodes BOOLEAN.FROMFLOAT and BOOL.DEFINE are not implemented as required FLOAT and NAME stacks (which are currently not implemented)
  • opcodes BOOLEAN.SHOVE and BOOLEAN.YANK are not implemented as required additional clarification and decision, if we are interested in those.
  • unit test coverage for edge cases and various stack configuration (seems like separate issue out of basic scope)

Breaking changes:

  • Changed code token format into combination/concatenation of ordered CodeTag and OpCode enums.
  • CodeTag is extended with BOOL_LITERAL. It moves SUBLIST (last CodeTag) and gives it new key: 0x04.
  • OpCode has empty key space for compatibility with CodeTag. It moves int opcodes and assigns new keys.
  • OpCode is extended with bool opcodes.
  • Added OPCODE offset constants to simplify ordering and execution flow.
  • Added INSTRUCTION recognition in parseSublist and implemented the simplest approach.

Required actions:

  • Review breaking changes, provide feedback. If approved, adjust backend code accordingly, if required.
  • Decide on INSTRUCTION in SUBLIST.
  • Review code comments related to BOOLEAN.SHOVE and BOOLEAN.YANK and provide additional clarification and feedback.

Concerns:

  • Unit tests are fragile on updating OpCode enum, would be cool to automate code generation to avoid it.
The goal of this PR is to implement bool stack and cover with unit tests to solve: - http://gitea.loseyourip.com:4000/giteadmin/SoLUSH-3/issues/11 - http://gitea.loseyourip.com:4000/giteadmin/SoLUSH-3/issues/12 - http://gitea.loseyourip.com:4000/giteadmin/SoLUSH-3/issues/13 I consider all issue are solved by this PR with minor tech dept. What is not solved: - opcodes BOOLEAN.FROMFLOAT and BOOL.DEFINE are not implemented as required FLOAT and NAME stacks (which are currently not implemented) - opcodes BOOLEAN.SHOVE and BOOLEAN.YANK are not implemented as required additional clarification and decision, if we are interested in those. - unit test coverage for edge cases and various stack configuration (seems like separate issue out of basic scope) Breaking changes: - Changed code token format into combination/concatenation of ordered CodeTag and OpCode enums. - CodeTag is extended with BOOL_LITERAL. It moves SUBLIST (last CodeTag) and gives it new key: 0x04. - OpCode has empty key space for compatibility with CodeTag. It moves int opcodes and assigns new keys. - OpCode is extended with bool opcodes. - Added OPCODE offset constants to simplify ordering and execution flow. - Added INSTRUCTION recognition in parseSublist and implemented the simplest approach. Required actions: - Review breaking changes, provide feedback. If approved, adjust backend code accordingly, if required. - Decide on INSTRUCTION in SUBLIST. - Review code comments related to BOOLEAN.SHOVE and BOOLEAN.YANK and provide additional clarification and feedback. Concerns: - Unit tests are fragile on updating OpCode enum, would be cool to automate code generation to avoid it.
navelbuffle added 17 commits 2025-01-05 06:25:03 +00:00
extend gitignore ce1cc8adad
extend CodeTag and OpCode enums with a boolean type 7d74d84717
extend Descriptor with a boolean type d38937095e
extend helper reads with a boolean type 0be8408a3a
refactored into ordered CodeTag and OpCode (same keys in a code) 9c657167a8
return to: all opcode implementations inline 68d9076424
refactor helper read funcs and add instruction into parseSublist ead3797ea0
added error ReadUintOutOfRange 3d7cf54723
fix opcode offsets in runInterpreter 7f259c8a07
add bool stack return and fix adapter e220ff39fc
add bool stack into adapter 79c8b3bd6d
fix interpeter unit tests 18063377fb
fix adapter unit tests 2e214f95d3
more clear offset values dfb7b933cc
basic bool operations implementation 818f702b32
implemented and tested majority of bool opcodes c86111206f
completed positive unit test coverage for available bool opcodes 39de432af4
navelbuffle requested review from giteadmin 2025-01-05 06:28:16 +00:00
navelbuffle requested review from GlowingOutlaw 2025-01-05 06:28:17 +00:00
GlowingOutlaw left a comment
Collaborator
Copy Link
Copy Source

Awesome work! The only issue I see, is that in tests we are preparing the stacks using the bytecode. I would prefer we prepare the stacks in the test outside the execution, and only really test the opcode at hand via bytecode. Makes for more isolated and easier to understand tests. Since navelbuffle is busy at work this could be done by someone else (like me hehe).

Awesome work! The only issue I see, is that in tests we are preparing the stacks using the bytecode. I would prefer we prepare the stacks in the test outside the execution, and only really test the opcode at hand via bytecode. Makes for more isolated and easier to understand tests. Since navelbuffle is busy at work this could be done by someone else (like me hehe).
onchain/src/Push3Interpreter.sol
@@ -318,0 +463,4 @@
(boolStack[boolTop - 1], boolStack[boolTop - 2], boolStack[boolTop - 3]) = (boolStack[boolTop - 3], boolStack[boolTop - 1], boolStack[boolTop - 2]);
}
}
else if (op == OpCode.BOOL_SHOVE) {
GlowingOutlaw commented 2025-01-08 04:19:15 +00:00
Collaborator
Copy Link
Copy Source

I think in such situations we should pick something that resembles the original instruction, but is easy on the gas. Like you suggested, we could just do a "deep swap". Swap the top element with the one at index.

I think in such situations we should pick something that resembles the original instruction, but is easy on the gas. Like you suggested, we could just do a "deep swap". Swap the top element with the one at index.
onchain/test/Push3Interpreter.t.sol
@@ -376,0 +413,4 @@
// We'll produce 8 total bytes, same structure
// Format: [0x04, 0x0005, 0x03(01), 0x03(00), 0x0C => bool_swap]
bytes memory code = hex"040005030103000C";
GlowingOutlaw commented 2025-01-08 04:12:40 +00:00
Collaborator
Copy Link
Copy Source

Here I would prefer to see a boolean stack with 2 elements loaded into the machine. Then we only do the swap op-code. Makes for a cleaner, more isolated test.

Here I would prefer to see a boolean stack with 2 elements loaded into the machine. Then we only do the swap op-code. Makes for a cleaner, more isolated test.
You are not authorized to merge this pull request.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin boolean-stack:boolean-stack
git checkout boolean-stack
Sign in to join this conversation.
Reviewers
No Reviewers
giteadmin
GlowingOutlaw
Labels
Clear labels
bounty-L
L / 900 $DUMP / effort ~13h
 bounty-M
M / 550 $DUMP / effort ~8h
 bounty-S
S / 350 $DUMP / effort ~5h
bounty-XL
XL / 1400 $DUMP / effort ~21h
 bounty-XS
XS / 200 $DUMP / effort ~ 3h
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
GlowingOutlaw navelbuffle VigilantFinder giteadmin
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: giteadmin/SoLUSH-3#18