Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RPC] Unify view types with StrView #979

Merged
merged 2 commits into from
Oct 16, 2023
Merged

[RPC] Unify view types with StrView #979

merged 2 commits into from
Oct 16, 2023

Conversation

jolestar
Copy link
Contributor

Summary

  1. Define H256View = StrView, H160View = StrView, H64 = StrView, IdentifierView = StrView
  2. Define BytesView = StrView<Vec>, and remove Bytes.
  3. Change u64 and u128 in RPC method arguments to StrView, StrView
  4. Remove net_version API and add eth namespace.
  5. Add StrView test cases.

@vercel
Copy link

vercel bot commented Oct 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
rooch ⬜️ Ignored (Inspect) Visit Preview Oct 16, 2023 8:17am

@jolestar jolestar merged commit 042bc09 into main Oct 16, 2023
4 checks passed
@jolestar jolestar deleted the str_view_unify branch October 16, 2023 09:00
@baichuan3
Copy link
Collaborator

baichuan3 commented Oct 16, 2023

Encounter an bad case after change u64 and u128 in RPC method arguments to StrView, StrView.

Either pass the integer type directly, or support both integer and string types?

curl --location 'http://localhost:50051' \
--header 'Content-Type: application/json' \
--data '{
    "id": 101,
    "jsonrpc": "2.0",
    "method": "rooch_getEventsByEventHandle",
    "params": [
        "0x4b1b5d2a62f3c9a19a5314167439945bf98fbc871e1402e36d5bad3382064c56::event_test::WithdrawEvent", null, 5
    ]
}'
{
    "jsonrpc": "2.0",
    "error": {
        "code": -32602,
        "message": "invalid type: integer `5`, expected a string at line 1 column 2"
    },
    "id": 101
}

@jolestar
Copy link
Contributor Author

Need to change 5 to "5"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Review] Use StrView<T> to replace H256View, H64View. etc
2 participants