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

Add exchange integration docs #10054

Merged
merged 7 commits into from
May 27, 2020

Conversation

CriesofCarrots
Copy link
Contributor

Problem

Now that we have some integrations online, it would be helpful to collect our accrued knowledge and make it actionable for future integration projects.

Summary of Changes

Add integrations section, and exchange integration page

Would appreciate some feedback on content and format: @mvines @jstarry
Are the rpc and cli examples useful or clutter? I largely skimmed over the JS sdk; should I be more explicit there?

@garious @mvines , it sounded like you had some ideas for the Testing section. Could you let me know what you were thinking?

Copy link
Member

@jstarry jstarry 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 so helpful to have all of this information in one place!

docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved

Optional parameters to consider:
- `--limit-ledger-size` specifies how many ledger shreds to retain on disk. If you do not include this parameter, the ledger will keep the entire ledger until it runs out of disk space. A larger value like `--limit-ledger-size 250000000000` is good for a couple days
- `--trusted-validator` can protect you from booting from a malicious snapshot. [More on the value of booting with trusted validators](../running-validator/validator-start.md#trusted-validators)
Copy link
Member

Choose a reason for hiding this comment

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

For avoid missing parts of the ledger, I think we should mention --no-snapshot-fetch. Maybe have a whole section on ledger continuity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Let me know if you have suggestions to make it more clear.

docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
If you need more information about the transaction type or other specifics, you
can request the block in binary format, and parse it using either our
[Rust SDK](https:/solana-labs/solana/tree/master/sdk) or
[Javascript SDK](https:/solana-labs/solana-web3.js).
Copy link
Member

Choose a reason for hiding this comment

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

FYI the javascript SDK doesn't support fetching binary blocks / transactions, but it does support decoding binary transactions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reworded the clause above to separate the request from the parsing. Let me know if that seems more clear.

docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
some SOL, they exist. To set up a deposit account for your exchange, simply
generate a Solana keypair using any of our [wallet tools](../wallet-guide/cli.md).

We recommend using a unique deposit account for each of your users.
Copy link
Member

Choose a reason for hiding this comment

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

We should mention that deposit accounts will be charged rent on first deposit and on epoch boundaries. Also, with eager rent collection, we will probably need a way to detect rent collections on accounts

Copy link
Member

Choose a reason for hiding this comment

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

Related issue for listening to rent changes for accounts: #10085

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll mention rent. I don't think rent is collected strictly on epoch boundaries, incidentally, but I don't think the current state of rent is documented anywhere :(
It would be nice to have to link to from this section.

Copy link
Member

Choose a reason for hiding this comment

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

Ugh, how about just creating a placeholder doc for rent that you can link to here, then a github issue for filling it in later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rent seemed like a natural continuation from app/README.md, so I put it first under that section, but doesn't seem consistent with the other pages in that menu. Open to other suggestions.

docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Show resolved Hide resolved
docs/src/integrations/exchange.md Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
docs/src/integrations/exchange.md Outdated Show resolved Hide resolved
@CriesofCarrots
Copy link
Contributor Author

@garious @mvines @jstarry , ready for review please

@CriesofCarrots
Copy link
Contributor Author

CriesofCarrots commented May 27, 2020

@garious @mvines , does this seem close enough to land? We can iterate on docs.

@mvines
Copy link
Member

mvines commented May 27, 2020

yep, this is a great start, thanks

@mvines mvines merged commit 7d42d52 into solana-labs:master May 27, 2020
mergify bot pushed a commit that referenced this pull request May 27, 2020
* Add exchange integration doc

* Round 1 review comments

* Add rent stub doc

* Pretty-print some things

* Rework blockhash info, move offline signing

* Add something to test section

* Update blockhash/last-valid-slot info

(cherry picked from commit 7d42d52)
CriesofCarrots added a commit that referenced this pull request May 27, 2020
* Add exchange integration doc

* Round 1 review comments

* Add rent stub doc

* Pretty-print some things

* Rework blockhash info, move offline signing

* Add something to test section

* Update blockhash/last-valid-slot info

(cherry picked from commit 7d42d52)
solana-grimes pushed a commit that referenced this pull request May 27, 2020
@CriesofCarrots CriesofCarrots deleted the add-exchange-doc branch May 28, 2020 23:29
danpaul000 pushed a commit to danpaul000/solana that referenced this pull request Jul 8, 2020
* Add exchange integration doc

* Round 1 review comments

* Add rent stub doc

* Pretty-print some things

* Rework blockhash info, move offline signing

* Add something to test section

* Update blockhash/last-valid-slot info
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.

4 participants