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

David/proposer offchain #549

Merged
merged 25 commits into from
Mar 10, 2022
Merged

David/proposer offchain #549

merged 25 commits into from
Mar 10, 2022

Conversation

druiz0992
Copy link
Contributor

@druiz0992 druiz0992 commented Mar 1, 2022

Fixes #524

This PR includes a proposer that exports two endpoints:

  • GET /healthcheck
  • POST /proposer/offchain-transaction to receive offchain transactions from users. The proposer will forward transactions received through this endpoint to optimist endpoint

Additional changes included are:

  • When a proposer registers, it now needs to provide its API URL so that the URL is recorded in the proposers contract. The proposer needs to have two endpoints implemented:
    • GET /healthcheck
    • POST /proposer/offchain-transaction
  • nightfall-client/src/routes/peers have been deleted, as the functionality is not used anymore. However, there is a function in nightfall-client/src/services/peers.mjs getProposers that return the next N proposers and the URLs so that a user can forward transactions directly. The URLs from proposers are extracted from proposers.sol contract directly.
  • Off chain transactions have been added to cli
  • Increased gas limit in proposer tests as a result of adding a url
  • New updateProposer(url) method added to SDK to update proposer's URL
  • Proposer can not be launched with two optional parameters
    • --proposerUrl : URL where API is located (http://172.16.238.1 by default, so that containers can communicate with it directly)
    • --proposerPort: Port where proposer is launched (8100 by default)

To test changes, simply run ping-pong test. If has been modified to alternate between onchain and offchain transactions. You can also test direct transactions with cli tools (launching ./nf and requesting offchain transactions).

NOTE: I had to merge PR with #514 because there were changes to ping pong test that were required.

@druiz0992 druiz0992 added the DNM Do not merge label Mar 1, 2022
@druiz0992 druiz0992 requested a review from Westlad March 1, 2022 13:56
@druiz0992 druiz0992 removed the DNM Do not merge label Mar 1, 2022
@signorecello
Copy link
Contributor

signorecello commented Mar 1, 2022

Hey the tests failed because it turns out we were really almost over the limit on gas. Add another 0 to the gas cost and they should run fine 😅

Done

config/default.js Outdated Show resolved Hide resolved
Copy link
Contributor

@ChaitanyaKonda ChaitanyaKonda left a comment

Choose a reason for hiding this comment

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

This is implemented fornightfall-client.
I'm assuming you planned implement this for wallet after wallet's merge into master?


I can't find where to answer this comment, so i just put my answer here. My idea was to have @IlyasRidhuan do it as he knows how the wallet is structured. I wouldn't know where to start

@ChaitanyaKonda
Copy link
Contributor

BREAKING CHANGE: Because registerProposer function signature changes

@druiz0992 druiz0992 added DNM Do not merge and removed Merge conflict labels Mar 4, 2022
@druiz0992 druiz0992 added the DNM Do not merge label Mar 7, 2022
@druiz0992 druiz0992 removed the DNM Do not merge label Mar 7, 2022
@druiz0992 druiz0992 added the One more approval needed One reviewer has approved this PR but another is needed label Mar 10, 2022
@ChaitanyaKonda ChaitanyaKonda removed the One more approval needed One reviewer has approved this PR but another is needed label Mar 10, 2022
@ChaitanyaKonda ChaitanyaKonda merged commit a52df0b into master Mar 10, 2022
@ChaitanyaKonda ChaitanyaKonda deleted the david/proposer-offchain branch March 10, 2022 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants