Various RPC discrepancies compared to Lotus

alt-ivan commented 2 weeks ago

Describe the bug

1) `StateSectorPreCommitInfo`

When sector precommit info is not found/non-existent, the RPC returns an error instead of null (as Lotus does). Expected (Lotus response):

{"jsonrpc":"2.0","result":null,"id":1}"

Actual (Forest):

{"jsonrpc":"2.0","error":{"code":-32603,"message":"SectorPreCommitOnChainInfo not found"},"id":1}

2) 'Message` deserialization has required fields that are optional in Lotus and default to 0 there

The fields

version
nonce
gas_limit
gas_fee_cap
gas_premium

are all optional when using Lotus (and will default to 0), whereas in Forest they are required and deserialization fails without them.

3) `StateWaitMsg` missing parameters

In Forest, this RPC only accepts two parameters and two are missing. Currently accepted parameters:

message_cid
confidence

Missing parameters:

limit: ChainEpoch
allow_replaced: bool

To summarize, Lotus accepts all 4 in that exact order:

message_cid
confidence
limit
allow_replaced

To reproduce

Call the related RPCs

Log output

Log Output

```Paste log output here paste log output... ```

Expected behaviour

Screenshots

Environment (please complete the following information):

OS:
Branch/commit
Hardware

Other information and links

hanabi1224 commented 2 weeks ago

I will look into this

hanabi1224 commented 2 weeks ago

Hey @alt-ivan Could you post the lotus version (lotus --version) you were testing against? It would also be great to have the payload you were using.

Regarding Filecoin.StateSectorPreCommitInfo, I get the below response when the sector number is invalid. (The lotus version I use locally is 1.27.2-dev+calibnet+git.6f821c328, and in CI 1.27.1-rc1-calibnet)
I've also added coverage for these cases to our API parity test suite in https://github.com/ChainSafe/forest/pull/4427

# payload
{"jsonrpc":"2.0","id":0,"method":"Filecoin.StateSectorPreCommitInfo","params":
["f04040",18446744073709551615,[{"/":"bafy2bzacednxz2tzqrneijbboehcrcbdt2357musif4dyetzyyjlvu6q6xnpq"},{"/":"bafy2bzaceblbbv22npiwjhqta3dmxxpzyedjrfxugsell3ouprocd3x333bqo"}]]
}
# response
{"error":{"code":1,"message":"precommit info does not exist"},"id":0,"jsonrpc":"2.0"}

alt-ivan commented 2 weeks ago

Hi @hanabi1224 ,

I am using this lotus image via docker-compose (though I suppose it could be considered very old at this point): ghcr.io/chainsafe/lotus-devnet:2024-04-07-d6d22ca

If there is parity with the newer version, I will update our codebase accordingly, and please ignore the first item :)

hanabi1224 commented 2 weeks ago

Update on 3) StateWaitMsg missing parameters

@alt-ivan could you describe how you tested this RPC method with 3 or 4 parameter against lotus? I tried to supply more than 2 parameters and got the below error

# lotus version
# lotus version 1.27.2-dev+calibnet+git.c87e2f2a6

# payload
{"jsonrpc":"2.0","id":0,"method":"Filecoin.StateWaitMsg","params":
[{"/":"bafy2bzaceb46hxz3jzwkgrztleojhad2eruk5dhyrz3ejqbgiyw4hzcp2dr4i"},0, 100]
}

# response
{"error":{"code":-32602,"message":"wrong param count (method 'Filecoin.StateWaitMsg'): 3 != 2"},"id":0,"jsonrpc":"2.0"}

According to the doc, it should accept exactly 2 parameters.

[ { "/": "bafy2bzacea3wsdh6y3a36tb3skempjoxqpuyompjbmfeyf34fi3uy6uue42v4" }, 42 ]

For the limit parameter, there is another Filecoin.StateWaitMessageLimited yet to be implemented in forest.

hanabi1224 commented 2 weeks ago

Update on 2) 'Message deserialization has required fields that are optional in Lotus and default to 0 there`

Fixed in https://github.com/ChainSafe/forest/pull/4440

alt-ivan commented 2 weeks ago

Update on 3) StateWaitMsg missing parameters

@alt-ivan could you describe how you tested this RPC method with 3 or 4 parameter against lotus? I tried to supply more than 2 parameters and got the below error
# lotus version
# lotus version 1.27.2-dev+calibnet+git.c87e2f2a6

# payload
{"jsonrpc":"2.0","id":0,"method":"Filecoin.StateWaitMsg","params":
[{"/":"bafy2bzaceb46hxz3jzwkgrztleojhad2eruk5dhyrz3ejqbgiyw4hzcp2dr4i"},0, 100]
}

# response
{"error":{"code":-32602,"message":"wrong param count (method 'Filecoin.StateWaitMsg'): 3 != 2"},"id":0,"jsonrpc":"2.0"}
According to the doc, it should accept exactly 2 parameters.

[ { "/": "bafy2bzacea3wsdh6y3a36tb3skempjoxqpuyompjbmfeyf34fi3uy6uue42v4" }, 42 ]

For the limit parameter, there is another Filecoin.StateWaitMessageLimited yet to be implemented in forest.

Were you running the RPCs against the v0 or v1 API version?

We are using StateWaitMsg, with V1 I just checked Lotus source and it appears all 4 parameters are there (latest commit on their releases branch - 2baca01fd8d8cac837800fbd8dfd39d96a407783 tag 1.27.0)

https://github.com/filecoin-project/lotus/blob/2baca01fd8d8cac837800fbd8dfd39d96a407783/api/api_full.go#L540

Though it seems they split it into two separate RPCs in some V1 "wrapper" whatever that is.

https://github.com/filecoin-project/lotus/blob/2baca01fd8d8cac837800fbd8dfd39d96a407783/api/v0api/v1_wrapper.go#L48

But I'm not sure how the API routing is exactly done on their end

EDIT: Yeah, it seems it's the other way around, it was split into 2 RPCs in V0 and is joined in V1: https://github.com/filecoin-project/lotus/blob/master/documentation/en/api-v1-unstable-methods.md#StateWaitMsg

The same might be true for item 1 of this issue

hanabi1224 commented 2 weeks ago

@alt-ivan, thanks for clarifying! Yes, I was testing the V0 API. Let me see what I can do for the V1 RPC methods.

hanabi1224 commented 1 week ago

@alt-ivan 1) and 3) have been fixed by #4443 and #4444 respectively. Thanks for reporting!

alt-ivan commented 6 days ago

@hanabi1224

I have tested and can confirm that the changes are working as expected! Thanks!

I will report two other discrepancies I've found here. If you want me to open another issue, please let me know. Both pertain to V1 of the Lotus API.

1) MpoolPending

expected response: empty json array result: [] if no messages found
actual response: result: null 2) StateSectorGetInfo
expected response: result: null if sector commit info not found

actual response:

    "error": {
    "code": -32603,
    "message": "Info for sector number 2 not found"
    },

hanabi1224 commented 1 day ago

Hey @alt-ivan MpoolPending and StateSectorGetInfo should have been fixed by #4481 and #4482 respectively. Thanks for reporting, as always!

ChainSafe / forest