ArkProjectNFTs / bridge

The ArkProject Bridge: seamless transfer of NFTs between ETH L1 & Starknet L2. Smart contracts, user-friendly interface, secure & efficient solution. Experience the future of NFT ownership today
https://bridge.arkproject.dev
Apache License 2.0
23 stars 20 forks source link

fix: handle mismatched types in newOwners/new_owners to prevent NFT loss #235

Closed SoarinSkySagar closed 1 month ago

SoarinSkySagar commented 1 month ago

Description

Closes issue #227.

Tasks:

This PR was created as part of OnlyDust's ODHack 8.0.

vercel[bot] commented 1 month ago

@SoarinSkySagar is attempting to deploy a commit to the Screenshot Team on Vercel.

A member of the Team first needs to authorize it.

SoarinSkySagar commented 1 month ago

Hi @ptisserand, I have completed all the required tasks and ensured good coding practices in the commit (Issue #227), please review and merge!

ptisserand commented 1 month ago

Hi, thanks for your contribution but there is a compilation issue when trying to run tests. Could you please fix it?

ptisserand commented 1 month ago

hi @ptisserand, yesterday when I ran the test it was compiling, can you please share the screenshot of the error so I can look into it?

Ok, maybe your forgot to commit & push, here is the errors:

--> src/TestMessaging.sol

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:125:9:
    |
125 |         buf1[0] = 0;
    |         ^^^^

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:126:93:
    |
126 |         (address[] memory result1, uint256 newOffset1) = Cairo.cairoAddressArrayDeserialize(buf1, 0);
    |                                                                                             ^^^^

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:131:9:
    |
131 |         buf2[0] = 2; 
    |         ^^^^

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:132:9:
    |
132 |         buf2[1] = uint160(address(0x1234567890abcdef1234567890abcdef12345678));
    |         ^^^^

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:133:9:
    |
133 |         buf2[2] = uint160(address(0xabcdefabcdefabcdefabcdefabcdefabcdefabcd));
    |         ^^^^

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:135:93:
    |
135 |         (address[] memory result2, uint256 newOffset2) = Cairo.cairoAddressArrayDeserialize(buf2, 0);
    |                                                                                             ^^^^

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:142:9:
    |
142 |         buf3[0] = 1; 
    |         ^^^^

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:143:9:
    |
143 |         buf3[1] = uint160(address(0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef));
    |         ^^^^

Error (7576): Undeclared identifier.
   --> test/Cairo.t.sol:145:93:
    |
145 |         (address[] memory result3, uint256 newOffset3) = Cairo.cairoAddressArrayDeserialize(buf3, 0);
    |                                                                                             ^^^^

Error: 
Compilation failed
SoarinSkySagar commented 1 month ago

Hi @ptisserand, can you please check now? Earlier the forge commands were running on the main branch of the forked repo whereas I was working on a new branch (even though I changed the branch to the working one), hence no errors were generated. Had to re-clone the forked repo and the errors were there as you said. Here's the new checklist of tasks completed:

Screenshot of compilation

image

SoarinSkySagar commented 1 month ago

Hi @ptisserand, hope its okay now and and merge-able :)