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

Enabling a custom client for Algod and Indexer #477

Merged
merged 2 commits into from
Dec 6, 2021

Conversation

fabrice102
Copy link
Contributor

Currently the only way for a dApp to use the SDK with an API service is to know the URL and token of the API service.
This prevents dApps from using API services / algod / indexer provided by the user or by a wallet such as AlgoSigner.

In particular:

(See also algorandfoundation/ARCs#8 for more details on wallets and discussion around them.)

The goal of this PR is to allow the above scenarios.
It does so by introducing an intermediate interface BaseHTTPClient and to allow to construct Algodv2Client and IndexerClient from such a BaseHTTPClient.

A BaseHTTPClient is minimal: it does not perform any serialization/de-serialization, does not fix accept headers, does not even try to utf8-decode strings. Its only purpose is to execute directly REST queries and return the raw responses.

BaseHTTPClient is meant to be stable over releases of algosdk.

Currently the only way for a dApp to use the SDK with an API service is to know the URL and token of the API service.
This prevents dApps from using API services / algod / indexer provided by the user or by a wallet such as AlgoSigner.

In particular:
* When using a wallet such as AlgoSigner, a dApp either need to manually serialize data and write path for endpoints (https://github.com/PureStake/algosigner/blob/develop/docs/dApp-integration.md#algosigneralgod-ledger-mainnettestnet-path-algod-v2-path--) or need to not use at all the provided API service.
* A user cannot use their preferred (potentially more trusted) API service / algod / index / API services
* A dApp cannot easily connect to a private network of the user

(See also algorandfoundation/ARCs#8 for more details on wallets and discussion around them.)

The goal of this PR is to allow the above scenarios.
It does so by introducing an intermediate interface `BaseHTTPClient` and to allow to construct `Algodv2Client` and `IndexerClient` from such a `BaseHTTPClient`.

A `BaseHTTPClient` is minimal: it does not perform any serialization/de-serialization, does not fix `accept` headers, does not even try to utf8-decode strings. Its only purpose is to execute directly REST queries and return the raw responses.

`BaseHTTPClient`  is meant to be stable over releases of algosdk.
@jasonpaulos jasonpaulos self-requested a review December 3, 2021 17:10
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

This generally looks pretty good, but I left a few comments/questions.

Additionally, you likely want to modify main.ts to export BaseHTTPClient and URLTokenBaseHTTPClient.

src/client/baseHTTPClient.ts Outdated Show resolved Hide resolved
src/client/baseHTTPClient.ts Outdated Show resolved Hide resolved
src/client/client.ts Show resolved Hide resolved
src/client/client.ts Outdated Show resolved Hide resolved
src/client/v2/algod/algod.ts Outdated Show resolved Hide resolved
@fabrice102
Copy link
Contributor Author

Thanks! I've done all the proposed changes.

Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

This looks good!

@jasonpaulos jasonpaulos merged commit 5f44fc5 into algorand:develop Dec 6, 2021
fabrice102 added a commit to fabrice102/ARCs that referenced this pull request Dec 7, 2021
From algorandfoundation#8
with the following changes:
* use of `genesisID`/`genesisHash` instead of `network` for `enable`
* enforce main networks' hashes from genesisID
* allow `enable` to return no account for just access to blockchain
* support of list of group of transactions in posting
* clarify error handling in many cases
* add security considerations in many places
* add optional `signTxns` and `postTxns` to wallet standard
* clarify requirements for confirmation when posting transactions
* use `BaseHTTPClient` from algorand/js-algorand-sdk#477
for `getAlgodv2Client` and `getIndexerClient`
* other minor changes

Main points to discuss:
* splitting the error standards out of ARC-0001 and make it a living standard
* allow optional argument to post without waiting for confirmation (dangerous)
aldur pushed a commit that referenced this pull request Jan 20, 2022
* Enabling a custom client for Algod and Indexer

Currently the only way for a dApp to use the SDK with an API service is to know the URL and token of the API service.
This prevents dApps from using API services / algod / indexer provided by the user or by a wallet such as AlgoSigner.

In particular:
* When using a wallet such as AlgoSigner, a dApp either need to manually serialize data and write path for endpoints (https://github.com/PureStake/algosigner/blob/develop/docs/dApp-integration.md#algosigneralgod-ledger-mainnettestnet-path-algod-v2-path--) or need to not use at all the provided API service.
* A user cannot use their preferred (potentially more trusted) API service / algod / index / API services
* A dApp cannot easily connect to a private network of the user

(See also algorandfoundation/ARCs#8 for more details on wallets and discussion around them.)

The goal of this PR is to allow the above scenarios.
It does so by introducing an intermediate interface `BaseHTTPClient` and to allow to construct `Algodv2Client` and `IndexerClient` from such a `BaseHTTPClient`.

A `BaseHTTPClient` is minimal: it does not perform any serialization/de-serialization, does not fix `accept` headers, does not even try to utf8-decode strings. Its only purpose is to execute directly REST queries and return the raw responses.

`BaseHTTPClient`  is meant to be stable over releases of algosdk.

* improvements followging review
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.

2 participants