Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#23931 closed enhancement (fixed)

Correcting Walsh Hadamard Transform in Boolean Function

Reported by: ruhm Owned by:
Priority: major Milestone: sage-8.1
Component: cryptography Keywords: BooleanFunction, SBox
Cc: jpflori, asante Merged in:
Authors: Rusydi H. Makarim Reviewers: Friedrich Wiemer
Report Upstream: N/A Work issues:
Branch: 408b4a9 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by ruhm)

This ticket fixes the incorrect computation of Walsh Hadamard Transform in BooleanFunction?. The incorrectness is due to wrong computation of sign function F corresponding to the BooleanFunction?, that is F(x) = 1 - 2*f(x) where f is an n-variable Boolean function with the set of integer as codomain. This issue is previously mentioned in the https://trac.sagemath.org/ticket/11450 .

Change History (14)

comment:1 Changed 5 years ago by ruhm

Component: PLEASE CHANGEcryptography
Description: modified (diff)
Keywords: BooleanFunction SBox added
Type: PLEASE CHANGEenhancement

comment:2 Changed 5 years ago by ruhm

Branch: u/ruhm/fix_walsh_transform

comment:3 Changed 5 years ago by ruhm

Cc: jpflori asante added
Commit: 54554db66bb4fcc2d03b6a61e3b83e1846474d34
Description: modified (diff)
Status: newneeds_review

New commits:

54554dbinitial commit

comment:4 Changed 5 years ago by ruhm

Authors: Rusydi H. Makarim

comment:5 Changed 5 years ago by asante

Reviewers: asante
Status: needs_reviewpositive_review

Looks good to me, thanks for fixing this!

As this is my first sage/trac review, maybe someone else also want to take a look. I checked (following this guide http://doc.sagemath.org/html/en/developer/reviewer_checklist.html#chapter-review ) that:

  • the code fixes the bug
  • the doctests touched by this patch pass
  • the reference doc builds without errors
  • all doctests (make ptestlong) pass (there are two failed tests in sage.tests.cmdline.test_executable, but these seem to be unrelated)

comment:6 Changed 5 years ago by vbraun

Status: positive_reviewneeds_work

Merge conflict

comment:7 Changed 5 years ago by git

Commit: 54554db66bb4fcc2d03b6a61e3b83e1846474d34408b4a9af3a7ca60e51a54c844a7e92c877e1e8b

Branch pushed to git repo; I updated commit sha1. New commits:

408b4a9Merge branch 'develop' of https://github.com/sagemath/sage into t/23931/fix_walsh_transform

comment:8 Changed 5 years ago by ruhm

Status: needs_workneeds_review

comment:9 Changed 5 years ago by asante

Status: needs_reviewpositive_review

lgtm

comment:10 Changed 5 years ago by chapoton

reviewer name should be a full name, not a login, please

comment:11 Changed 5 years ago by asante

Reviewers: asanteFriedrich Wiemer

sorry, fixed this.

comment:12 Changed 5 years ago by vbraun

Branch: u/ruhm/fix_walsh_transform408b4a9af3a7ca60e51a54c844a7e92c877e1e8b
Resolution: fixed
Status: positive_reviewclosed

comment:13 Changed 5 years ago by ruhm

Commit: 408b4a9af3a7ca60e51a54c844a7e92c877e1e8b

Although the milestone is set to sage-8.1, the patch in this ticket is not merged in version 8.1. Should I reopen this ticket and change the milestone to 8.2 ? Or is it sufficient to change only the milestone in ticket description ?

Regards, Rusydi

comment:14 Changed 5 years ago by chapoton

just do nothing

Note: See TracTickets for help on using tickets.