contract: add erc20 functions #100

kroist · 2025-01-30T13:33:25Z

No description provided.

github-actions · 2025-01-31T15:46:15Z

Transaction Name	Main	Current	Difference (%)
NewAccount	2001147	2001147	0.00000%
Deposit	1828664	1828664	0.00000%
Withdraw	1898683	1898683	0.00000%

github-actions · 2025-01-31T15:51:50Z

📊 Coverage Report

📈 Total Coverage Summary

Type	Covered	Total	Coverage
📝 Lines	2008	3151	🟠 63.72%
📄 Statements	2008	3151	🟠 63.72%
⚡ Functions	69	77	🟡 89.61%
🔀 Branches	197	200	🟡 98.5%

Coverage Legend

✅ 100% Coverage
🟡 80-99% Coverage
🟠 50-79% Coverage
❌ 0-49% Coverage

📁 File Coverage

📋 Detailed Coverage Report

File	Lines	Statements	Functions	Branches
✅ zkOS-monorepo/ts/shielder-sdk/src/_generated/abi_v0_0_1.ts	100%	100%	100%	100%
❌ zkOS-monorepo/ts/shielder-sdk/src/_generated/abi.ts	0%	0%	0%	0%
✅ zkOS-monorepo/ts/shielder-sdk/src/actions/deposit.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/actions/index.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/actions/newAccount.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/actions/utils.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/actions/withdraw.ts	100%	100%	100%	100%
❌ zkOS-monorepo/ts/shielder-sdk/src/chain/contract.ts	9.09%	9.09%	25%	100%
❌ zkOS-monorepo/ts/shielder-sdk/src/chain/relayer.ts	20.75%	20.75%	33.33%	50%
✅ zkOS-monorepo/ts/shielder-sdk/src/client.ts	100%	100%	93.75%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/constants.ts	100%	100%	100%	100%
❌ zkOS-monorepo/ts/shielder-sdk/src/index.ts	0%	0%	0%	0%
✅ zkOS-monorepo/ts/shielder-sdk/src/state/events.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/state/index.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/state/manager.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/state/storageSchema.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/state/sync.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/state/types.ts	100%	100%	100%	100%
✅ zkOS-monorepo/ts/shielder-sdk/src/utils.ts	100%	100%	100%	100%

pmikolajczyk41 · 2025-02-03T16:02:46Z

.github/workflows/_measure-gas.yml

a lot of commented jobs - I guess this is something to be restored

Assuming it's doable in this PR, not sure what breaks if we restore it now. We might prefer leaving a TODO and/or JIRA (sub)task.

I commented it cause the new binary for gas measurement is not compatible with main branch contracts
https://github.com/Cardinal-Cryptography/zkOS-monorepo/actions/runs/13071400613/job/36473677457

This can be fixed with compiling the additional binary on the main branch. Not sure if it's worth doing, what do you suggest?

Any decision that ensures we don't forget about restoring the commented-out part is fine by me. Leaving this for @pmikolajczyk41 to weigh in.

this PR is already huge; I'd suggest adding Jira ticket and solving this in a follow-up

https://cardinal-cryptography.atlassian.net/browse/ZK-721

contracts/DepositLimit.sol

contracts/Shielder.sol

crates/shielder-account/src/shielder_action.rs

guspiel

Awesome!

Haven't read everything, but posting some initial comments. Will finish reviewing today.

crates/shielder-cli/src/recovery.rs

contracts/Shielder.sol

guspiel · 2025-02-04T13:51:45Z

.github/workflows/_measure-gas.yml

Assuming it's doable in this PR, not sure what breaks if we restore it now. We might prefer leaving a TODO and/or JIRA (sub)task.

crates/integration-tests/src/shielder/calls/deposit_native.rs

contracts/Shielder.sol

crates/shielder-contract/src/types.rs

crates/shielder-account/src/shielder_action.rs

contracts/DepositLimit.sol

contracts/Shielder.sol

crates/integration-tests/src/shielder/calls/new_account_native.rs

contracts/Shielder.sol

pmikolajczyk41

nice work, kudos!

for future, I'd suggest doing preliminary, no-brainer PR for simply renaming contract functions and applying this to >20 files, and then, proper PR with semantic changes

pmikolajczyk41 · 2025-02-06T16:19:15Z

contracts/Shielder.sol

-    bytes3 public constant CONTRACT_VERSION = 0x000001;
+    bytes3 public constant CONTRACT_VERSION = 0x000100;


just out of curiosity - have we already decided that the new version is 0.1.0, not 0.0.2? (apart from this PR ofc :P)

The agreement was that if circuits get updated, the middle (or the first) number gets bumped.

pmikolajczyk41 · 2025-02-06T16:25:54Z

contracts/Shielder.sol

@@ -313,4 +323,40 @@ contract Shielder is
    function setDepositLimit(uint256 _depositLimit) external onlyOwner {
        _setDepositLimit(_depositLimit);
    }
+
+    function handleTokenTransferIn(


+1 for these handlers

pmikolajczyk41 · 2025-02-06T16:26:39Z

contracts/Shielder.sol

+        } else {
+            IERC20 token = IERC20(tokenAddress);
+            bool transferSuccess = token.transfer(to, amount);
+            if (!transferSuccess) revert NativeTransferFailed();


new error is needed here (not native transfer)

added ERC20TransferFailed

guspiel

LGTM

guspiel · 2025-02-07T10:13:33Z

.github/workflows/_measure-gas.yml

https://cardinal-cryptography.atlassian.net/browse/ZK-721

kroist added 8 commits January 30, 2025 14:33

add erc20 functions

42a2a32

integration tests

e29fb41

upd

b636681

upd

bfc409b

upd

81db006

upd

e69f681

upd

1b75faf

upd

661a3fe

upd

9158af7

kroist marked this pull request as ready for review February 3, 2025 09:07

kroist requested review from guspiel and pmikolajczyk41 February 3, 2025 11:35

kroist changed the title ~~add erc20 functions~~ contract: add erc20 functions Feb 3, 2025

pmikolajczyk41 reviewed Feb 3, 2025

View reviewed changes

guspiel reviewed Feb 4, 2025

View reviewed changes

kroist added 4 commits February 5, 2025 13:44

upd

3bb9b0c

upd

8328650

fix

e273fa3

upd

3f63971

pmikolajczyk41 approved these changes Feb 6, 2025

View reviewed changes

upd

9e6c916

guspiel approved these changes Feb 7, 2025

View reviewed changes

.github/workflows/_measure-gas.yml Outdated

Copy link

Contributor

guspiel Feb 7, 2025

Choose a reason for hiding this comment

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

https://cardinal-cryptography.atlassian.net/browse/ZK-721

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

contract: add erc20 functions #100

contract: add erc20 functions #100

kroist commented Jan 30, 2025

github-actions bot commented Jan 31, 2025 •

edited

Loading

github-actions bot commented Jan 31, 2025 •

edited

Loading

pmikolajczyk41 Feb 3, 2025

guspiel Feb 4, 2025

kroist Feb 4, 2025

guspiel Feb 4, 2025

pmikolajczyk41 Feb 6, 2025

guspiel Feb 7, 2025

guspiel left a comment

guspiel Feb 4, 2025

pmikolajczyk41 left a comment

pmikolajczyk41 Feb 6, 2025

guspiel Feb 6, 2025

pmikolajczyk41 Feb 6, 2025

pmikolajczyk41 Feb 6, 2025

kroist Feb 7, 2025

guspiel left a comment

guspiel Feb 7, 2025

		bytes3 public constant CONTRACT_VERSION = 0x000001;
		bytes3 public constant CONTRACT_VERSION = 0x000100;

contract: add erc20 functions #100

Are you sure you want to change the base?

contract: add erc20 functions #100

Conversation

kroist commented Jan 30, 2025

github-actions bot commented Jan 31, 2025 • edited Loading

github-actions bot commented Jan 31, 2025 • edited Loading

📊 Coverage Report

📈 Total Coverage Summary

Coverage Legend

📁 File Coverage

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guspiel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmikolajczyk41 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guspiel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 31, 2025 •

edited

Loading

github-actions bot commented Jan 31, 2025 •

edited

Loading