Skip to content

Conversation

Boog900
Copy link
Member

@Boog900 Boog900 commented Nov 1, 2024

What

Makes changes to some crates to support 32 bit systems for rpc/types

Why

WASM

@github-actions github-actions bot added A-dependency Area: Related to dependencies, or changes to a Cargo.{toml,lock} file. A-workspace Area: Changes to a root workspace file or general repo file. A-net Area: Related to networking. A-rpc Area: Related to RPC. labels Nov 1, 2024
@spirobel
Copy link

spirobel commented Nov 3, 2024

lgtm 👍

I managed to construct a request object for get_blocks.bin with this branch.

some observations regarding the types:
Afterwards i passed the request object to typescript and used it in the body of the fetch request. The response was then passed back to rust and parsed into a GetBlocksResponse object.

First I used GetBlocksRequest::default() to construct the request which meant the start_height was set to 0.

As a result the finish function gave me: "Required field was not found!"

fn finish(self) -> error::Result<GetBlocksResponse> {

This is because there are no Blocks in this response.
Afterwards I set the start_height to 100. So the response contained blocks.
The result was still: "Required field was not found!"

This time this was because of

  let pool_info_extent = self.pool_info_extent.ok_or(ELSE)?; 

After I manually inserted

 let pool_info_extent = PoolInfoExtent::None;

the Response was parsed successfully.

Suggestions:

  1. Empty blocks should be a valid GetBlocksResponse object.
  2. pool_info_extent should be set to None instead of throwing an error
  3. the error message should indicate which field is missing.

EDIT:
same applies to daemon_time. Both of these values get turned into None https://github.com/monero-project/monero/blob/893916ad091a92e765ce3241b94e706ad012b62a/src/rpc/core_rpc_server_commands_defs.h#L265 (macro reference https://github.com/monero-project/monero/blob/893916ad091a92e765ce3241b94e706ad012b62a/contrib/epee/include/serialization/keyvalue_serialization.h#L90)

@github-actions github-actions bot added A-ci Area: Related to CI. and removed A-rpc Area: Related to RPC. labels Jan 7, 2025
@Boog900 Boog900 marked this pull request as ready for review January 7, 2025 03:38
@Boog900 Boog900 requested a review from hinto-janai January 7, 2025 03:38
Co-authored-by: hinto-janai <hinto.janai@protonmail.com>
@Boog900 Boog900 merged commit 79e4789 into main Jan 8, 2025
9 checks passed
@Boog900 Boog900 deleted the epee-32-bit branch January 14, 2025 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ci Area: Related to CI. A-dependency Area: Related to dependencies, or changes to a Cargo.{toml,lock} file. A-net Area: Related to networking. A-workspace Area: Changes to a root workspace file or general repo file.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants