Skip to content
/ zuc Public
generated from Nugine/rust-template

128-EEA3 implementation#4

Merged
Nugine merged 21 commits intoNugine:mainfrom
jackyliu16:main
Dec 15, 2024
Merged

128-EEA3 implementation#4
Nugine merged 21 commits intoNugine:mainfrom
jackyliu16:main

Conversation

@jackyliu16
Copy link
Contributor

@jackyliu16 jackyliu16 commented Dec 10, 2024

implementation EEA-128 algorithm, based on GB/T 33133.2-2021

@Nugine Nugine self-requested a review December 10, 2024 14:35
@Nugine
Copy link
Owner

Nugine commented Dec 10, 2024

CC @Xerxes-zhz

@Nugine
Copy link
Owner

Nugine commented Dec 10, 2024 •
edited
Loading

可参考 GmSSL 的实现，对比每一步，看哪里有问题

入口点

gm-zuc

@jackyliu16
Copy link
Contributor Author

jackyliu16 commented Dec 10, 2024

可参考 GmSSL 的实现，对比每一步，看哪里有问题

入口点

* https://github.com/guanzhi/GmSSL/blob/34fa519dc0f94a9a3995d9daf09c84cdac37abd8/src/zuc_modes.c#L32-L49

gm-zuc

* https://github.com/CrayfishGo/gm-rs/blob/main/gm-zuc/src/eea.rs

好的,我去研究一下
目前在跟这个对比 git@github.com:guyongqiangx/crypto-work.git (push)

@Nugine Nugine mentioned this pull request Dec 10, 2024
Merged
@Nugine Nugine removed their request for review December 10, 2024 15:55
@jackyliu16 jackyliu16 force-pushed the main branch 6 times, most recently from ee28400 to cd55bba Compare December 11, 2024 05:39
Nugine
Nugine requested changes Dec 11, 2024
View reviewed changes
Copy link
Owner

@Nugine Nugine left a comment

Choose a reason for hiding this comment

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

主要建议

  • review 中提出的修改要求
  • 尝试减少 eea3_128 的内存分配次数，最好仅需单次 Vec 分配，无需反复扩容和多次创建

@Xerxes-zhz 若 #4 先合并的话可能与 #3 产生冲突，你也看一下有没有其他问题

@Nugine Nugine self-requested a review December 11, 2024 07:12
Nugine
Nugine requested changes Dec 11, 2024
View reviewed changes
@Nugine Nugine self-requested a review December 12, 2024 09:42
Nugine
Nugine requested changes Dec 13, 2024
View reviewed changes
Copy link
Owner

@Nugine Nugine left a comment •
edited
Loading

Choose a reason for hiding this comment

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

剩余工作

  • rebase 到 main，解决冲突，并修复 lint 报错
  • 解决 review 要求

完成之后，若准备好合并，发一条 Ready to merge 消息即可

@Nugine Nugine changed the title eea-128 implementation 128-EEA3 implementation Dec 13, 2024
Nugine
Nugine reviewed Dec 13, 2024
View reviewed changes
@jackyliu16
Copy link
Contributor Author

jackyliu16 commented Dec 14, 2024

Ready to merge

Nugine
Nugine requested changes Dec 15, 2024
View reviewed changes
Copy link
Owner

@Nugine Nugine left a comment •
edited
Loading

Choose a reason for hiding this comment

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

剩余任务：

  • 看 CI lint 报错，修复 tests 中相关格式问题
  • 解决 review 要求

我们希望代码质量尽可能做到最优，很多高要求的开源项目 PR 都会经过反复多轮修改，长 review 周期很常见。这也是一次练习 Rust 的机会，慢慢来。

@jackyliu16
Copy link
Contributor Author

jackyliu16 commented Dec 15, 2024 •
edited
Loading

有点难绷了, 你这一个接一个的, 然后这 clippy 天天莫名其妙的问题
要我 0x33532455 改成 0x0x3353_2455 就算了, 然后过长还给我自动格式化成单列然后报错函数太长
我自己处理 nixos 的 review 都没那么多事情.

修改完你的意见心情都不好了属于是

@Nugine
Copy link
Owner

Nugine commented Dec 15, 2024

抱歉。这次活动是我第一次试验带奖励的 PR 模式，对代码质量的期待可能过高，后续将会调整。
对于不合理的 lint 要求，我将自己尝试后再提出，或者直接自行修改。
对于代码逻辑和文档的要求，应更清晰明确，给出必要的提示。

由于您未能达成单次内存分配的 review 要求，最终奖励将有扣减。
此次 PR 我已自行修改到符合要求。如果您仍对其他 todo item 感兴趣，欢迎继续参与奖励活动。

@Nugine Nugine merged commit 469f088 into Nugine:main Dec 15, 2024
3 checks passed
@Nugine Nugine mentioned this pull request Dec 24, 2024
Merged
Copilot AI mentioned this pull request Dec 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nugine Nugine Nugine requested 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

@jackyliu16 @Nugine