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

AMM RPC endpoints for UI integration #220

Merged
merged 21 commits into from
Apr 24, 2023
Merged

AMM RPC endpoints for UI integration #220

merged 21 commits into from
Apr 24, 2023

Conversation

RustNinja
Copy link
Contributor

@RustNinja RustNinja commented Apr 19, 2023

PR related to #217
and pendulum-chain/Zenlink-DEX-Module#1

PR based on top of this PR(it is already merged)

  • expose zenlink rpc endpoints from pendulum/amplitude/foucoco runtime

@ebma
Copy link
Member

ebma commented Apr 20, 2023

@TorstenStueber in case you are wondering why we need yet another fork, I looked into this with Nikita and we tried to patch it again but it did not work unfortunately. Forking was easiest, and we can get rid of that fork again, once we complete #219 because Zenlink has more up-to-date jsonrpsee dependencies on their polkadot-v0.9.38 branch.

RustNinja and others added 4 commits April 21, 2023 13:33
207 implement lp rewards on amm (#208)

* add bifrost-farming and bifrost-farming-rpc-runtime-api dependency

* introduce T::CurrencyId to bifrost farming pallet

* add farming runtime rpc to foucoco

* Update runtime/foucoco/src/lib.rs



* fix build in foucoco runtime with FoucocoTreasuryAccount

* add patch for git and crates io

* update pr according to request changes in PR

#208

* Add farming rpc api intou pendulum node for foucoco runtime (#216)

add farming rpc api for foucoco

* move PoolId type declaration to runtime common

* renaming

create_full_spacewalk_foucoco -> create_full_foucoco
create_full_spacewalk -> create_full_amplitude
create_full -> create_full_pendulum

* replace "introduce-abstraction-for-currency-id" to "polkadot-v0.9.37"

* Update runtime/common/src/lib.rs



* renaming 

start_node_impl() -> start_node_impl_pendulum()
start_node_impl_spacewalk_foucoco() -> start_node_impl_foucoco()
start_node_impl_spacewalk_amplitude() -> start_node_impl_amplitude()

---------

Co-authored-by: Marcel Ebert <[email protected]>
C::Api: ZenlinkProtocolRuntimeApi<Block, AccountId, AssetId>,
node/src/rpc.rs Outdated Show resolved Hide resolved
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said in the comments I would not introduce a separate development config now. At least not as part of this PR. If this is needed then let's do that somewhere else, after discussing it

node/src/rpc.rs Show resolved Hide resolved
node/src/command.rs Show resolved Hide resolved
@RustNinja RustNinja requested a review from ebma April 21, 2023 14:23
@RustNinja
Copy link
Contributor Author

As I said in the comments I would not introduce a separate development config now. At least not as part of this PR. If this is needed then let's do that somewhere else, after discussing it

@ebma
it is not possible to NOT introduce new function for development because of the different implementation between pendulum runtime and development runtime.
Now to expose Zenlink RPC endpoints we need to add new trait bound
C::Api: ZenlinkProtocolRuntimeApi<Block, AccountId, AssetId>,

it means that it not possible anymore to use the same function as for pendulum and development runtime.

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I see. I was not fully aware what's added to the development runtime. AFAIK we don't really use it. But it's probably the right step to also add other functions for that, now that we basically split everything up per network anyways.

The rest LGTM.

@TorstenStueber
Copy link
Contributor

@TorstenStueber in case you are wondering why we need yet another fork, I looked into this with Nikita and we tried to patch it again but it did not work unfortunately. Forking was easiest, and we can get rid of that fork again, once we complete #219 because Zenlink has more up-to-date jsonrpsee dependencies on their polkadot-v0.9.38 branch.

I am a little confused about the issues and PRs and how they are linked. This PR here is linked with an issue (#217), which itself is linked to another PR (#218), then you mention issue #219.

Is there a way to clean up the PR <-> issue structure?

@RustNinja RustNinja linked an issue Apr 24, 2023 that may be closed by this pull request
@RustNinja
Copy link
Contributor Author

@TorstenStueber in case you are wondering why we need yet another fork, I looked into this with Nikita and we tried to patch it again but it did not work unfortunately. Forking was easiest, and we can get rid of that fork again, once we complete #219 because Zenlink has more up-to-date jsonrpsee dependencies on their polkadot-v0.9.38 branch.

I am a little confused about the issues and PRs and how they are linked. This PR here is linked with an issue (#217), which itself is linked to another PR (#218), then you mention issue #219.

Is there a way to clean up the PR <-> issue structure?

@TorstenStueber
Done

@RustNinja RustNinja merged commit ebd6e55 into main Apr 24, 2023
@RustNinja RustNinja deleted the zenlink-rpc branch April 24, 2023 17:24
@@ -766,6 +771,192 @@ where
Ok((task_manager, client))
}

/// Start a node with the given parachain `Configuration` and relay chain `Configuration`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RustNinja where is all of this code coming from? Is the code duplication now even getting worse?

If it is just duplicated code, then fine. If there are any crucial changes to the duplicated code here, can you point them out?

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.

Add Zenlink RPC to pendulum node
3 participants