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 call uses lowercase parameter name #7645

Closed
gregdhill opened this issue May 26, 2022 · 3 comments
Closed

RPC call uses lowercase parameter name #7645

gregdhill opened this issue May 26, 2022 · 3 comments
Labels
support wontfix This will not be worked on

Comments

@gregdhill
Copy link
Contributor

See the example in interlay/interbtc#612

We have an RPC call which uses a CurrencyId argument, note the capitalized "Token" enum variant (see here). I can only assume polkadot-js uses the lowercase of this string which results in the "Invalid params" error as the correct capitalization works as expected. We do not have any issues with extrinsics or reading the chain state.

@jacogr
Copy link
Member

jacogr commented May 26, 2022

As per the Substrate de-facto standard, all properties over RPC are camelCased, e.g.#[serde(rename_all = "camelCase")] is added everywhere RPCs are exposed.

As an example from Substrate (see the serde macro) -

/// Health struct returned by the RPC
#[derive(Debug, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct Health {
	/// Number of connected peers
	pub peers: usize,
	/// Is the node syncing
	pub is_syncing: bool,
	/// Should this node have any peers
	///
	/// Might be false for local chains or when running without discovery.
	pub should_have_peers: bool,
}

The same would apply to enums, e.g. an example from Substrate -

/// Possible transaction status events.
///
/// This events are being emitted by `TransactionPool` watchers,
/// which are also exposed over RPC.
/// ...
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub enum TransactionStatus<Hash, BlockHash> {
	/// Transaction is part of the future queue.
	Future,
	/// Transaction is part of the ready queue.
	Ready,
	/// The transaction has been broadcast to the given peers.
	Broadcast(Vec<String>),
	/// Transaction has been included in block with given hash.
	InBlock(BlockHash),
	/// The block this transaction was included in has been retracted.
	Retracted(BlockHash),
	/// Maximum number of finality watchers has been reached,
	/// old watchers are being removed.
	FinalityTimeout(BlockHash),
	/// Transaction has been finalized by a finality-gadget, e.g GRANDPA
	Finalized(BlockHash),
	/// Transaction has been replaced in the pool, by another transaction
	/// that provides the same tags. (e.g. same (sender, nonce)).
	Usurped(Hash),
	/// Transaction has been dropped from the pool because of the limit.
	Dropped,
	/// Transaction is no longer valid in the current state.
	Invalid,
}

This is what the API codes to and decodes from. (although in decoding it is more lenient, unlike Substrate which is very ridgit with the requirements). Trying to change this would mean all the base Substrate RPCs break. So the requirement would be to follow the Substrate standards and add the macro to your code if you want it to go over RPC.

TL:DR Add the required macros to the Rust code on everything exposed via RPC as per the Substrate de-facto standard (thus following the groundwork already laid by the Substrate team)

@jacogr jacogr added support wontfix This will not be worked on labels May 26, 2022
@gregdhill
Copy link
Contributor Author

Thanks for the detailed answer @jacogr, much appreciated!

@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
support wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants