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

Typo and bug fixes for the ACC instruction family #462

Merged
merged 3 commits into from
Oct 20, 2024

Conversation

OlivierBBB
Copy link
Collaborator

@OlivierBBB OlivierBBB commented Oct 17, 2024

  • more shorthands, fewer [ stack/DEC_FLAG k ]'s and [ stack/STACK_ITEM_VALUE k ]'s
  • boolean flag per instruction
  • using the account-instruction---bla convention
  • named constants for row offsets
  • splitting of some constraints
  • force account/ROM_LEX_FLAG = 0 whenever possible
  • changed the NSR of instructions that touch a current account (CODESIZE, SELFBALANCE)
  • Bug-fix: SELFBALANCE was receiving the wrong value
  • Bug-fix: several shorthands were missing shift's ... !

- more shorthands, fewer [ stack/DEC_FLAG k ]'s and [
stack/STACK_ITEM_VALUE k ]'s
- boolean flag per instruction
- using the account-instruction---bla convention
- constants for row offsets
- splitting of constraints
- bug-fix: SELFBALANCE was receiving the wrong value
- bug-fix: several shorthands were missing shifts ... !
@OlivierBBB OlivierBBB added bug Something isn't working veridise audit labels Oct 17, 2024
@OlivierBBB OlivierBBB self-assigned this Oct 17, 2024
@OlivierBBB OlivierBBB linked an issue Oct 17, 2024 that may be closed by this pull request

(defun (account-instruction---no-trimming) (- stack/ACC_FLAG (account-instruction---touches-foreign-account)))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed the no-trimming shorthand in favour of the touches-current-account shorthand

(account-turn-on-warmth ROW_OFFSET___ACC_FAMILY___ACCOUNT_READING_ROW)
(account-same-marked-for-selfdestruct ROW_OFFSET___ACC_FAMILY___ACCOUNT_READING_ROW)
(DOM-SUB-stamps---standard ROW_OFFSET___ACC_FAMILY___ACCOUNT_READING_ROW 0)
(read-context-data ROFF_ACC___CONTEXT_ROW CONTEXT_NUMBER)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using constant for 1

(if-not-zero (account-instruction---is-SELFBALANCE)
(begin
(eq! (account-instruction---result-hi) 0)
(eq! (account-instruction---result-lo) (account-instruction---current-balance)))))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Splitting of the account-instruction-value-constraints constraint and using shorthands to keep things concrete.

(if-not-zero (- 1 (account-instruction-decoded-flags-sum))
(begin
(eq! [ stack/STACK_ITEM_VALUE_HI 4 ] 0)
(eq! [ stack/STACK_ITEM_VALUE_LO 4 ] (shift account/CODE_SIZE 2))))))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bug fix. We were using the code size rather than the balance as the return value of SELFBALANCE.

(defun (account-instruction---account-address-lo) (shift context/ACCOUNT_ADDRESS_LO ROFF_ACC___CONTEXT_ROW))
(defun (account-instruction---byte-code-address-hi) (shift context/BYTE_CODE_ADDRESS_HI ROFF_ACC___CONTEXT_ROW))
(defun (account-instruction---byte-code-address-lo) (shift context/BYTE_CODE_ADDRESS_LO ROFF_ACC___CONTEXT_ROW))
(defun (account-instruction---foreign-address-warmth) (shift account/WARMTH ROFF_ACC___ACCOUNT_DOING_ROW))
Copy link
Collaborator Author

@OlivierBBB OlivierBBB Oct 17, 2024

Choose a reason for hiding this comment

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

Bug fix. These shorthands for values in context/ or account/ rows were missing a shift !

@OlivierBBB
Copy link
Collaborator Author

@letypequividelespoubelles all modifications that matter have a comment.

@OlivierBBB OlivierBBB changed the title Typo fixes for the ACC instruction family Typo fixes for the ACC instruction family Oct 17, 2024
@OlivierBBB OlivierBBB changed the title Typo fixes for the ACC instruction family Typo and bug fixes for the ACC instruction family Oct 17, 2024
- splitting of constraints according to foreign-address-opcode or
current-address-opcode
- updated NSR constraints
- added vanishing constraints for account/ROM_LEX_FLAG's
- renaming of constraints
Copy link
Collaborator

@letypequividelespoubelles letypequividelespoubelles left a comment

Choose a reason for hiding this comment

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

LGTM, only cosmetic comment

(defun (account-instruction---is-EXTCODESIZE) [ stack/DEC_FLAG 2 ] )
(defun (account-instruction---is-EXTCODEHASH) [ stack/DEC_FLAG 3 ] )
(defun (account-instruction---is-CODESIZE) [ stack/DEC_FLAG 4 ] ) ;; ""
(defun (account-instruction---is-SELFBALANCE) (- 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

for clarity, maybe better (- 1 (+ _sum shorthands)) ? instead of (- 1 \sum shorthands)

@OlivierBBB OlivierBBB merged commit 2f52939 into master Oct 20, 2024
2 checks passed
@OlivierBBB OlivierBBB deleted the 461-acc-family-constraints-review branch October 20, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working veridise audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ACC family constraints review
2 participants