Zondax / filecoin-solidity

Filecoin Solidity API Library
Apache License 2.0
95 stars 43 forks source link

deserializeAuthenticateMessageParams is never used #339

Closed ainhoa-a closed 1 year ago

ainhoa-a commented 1 year ago

Please remove the deserializeAuthenticateMessageParams function from the cbor/AccountCbor.sol.

The deserializeAuthenticateMessageParams in the AccountCBOR library is never called:

function deserializeAuthenticateMessageParams(bytes memory rawResp) internal pure returns (AccountTypes.AuthenticateMessageParams memory ret) {
    uint byteIdx = 0;
    uint len;

    (len, byteIdx) = rawResp.readFixedArray(byteIdx);
    assert(len == 2);

    (ret.signature, byteIdx) = rawResp.readBytes(byteIdx);
    (ret.message, byteIdx) = rawResp.readBytes(byteIdx);

    return ret;
}

As the function deserializes the parameters passed to the account actor’s AuthenticateMessage method it looks like a leftover used for testing. Furthermore, the AuthenticateMessage method returns a bool value, so the deserializeAuthenticateMessageParams is not even compatible with the returned value from the actor.

:link: zboto Link

shrenujbansal commented 1 year ago

Hi @ainhoa-a, We are currently using this deser API in our client contract. This will also be needed for anyone else trying to make deal making contracts since its how they would deserialize Authenticate messages coming from the market actor If you have a different pattern/tool available for the above problem, I'm happy to review that and move to using it instead

Furthermore, the AuthenticateMessage method returns a bool value, so the deserializeAuthenticateMessageParams is not even compatible with the returned value from the actor

This functionality deserializes the input params to the AuthenticateMessage method. The bool return value is a separate concern from this