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 polling to sep6 #79

Merged
merged 1 commit into from
Nov 29, 2023
Merged

add polling to sep6 #79

merged 1 commit into from
Nov 29, 2023

Conversation

acharb
Copy link
Contributor

@acharb acharb commented Nov 28, 2023

ticket

re-using a lot of the same logic already there for polling sep24 transactions

@@ -212,38 +214,20 @@ export class Sep24 {
const toml = await this.anchor.sep1();
const transferServerEndpoint = toml.transferServerSep24;

let qs: { [name: string]: string } = {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

basically just moving this code to a helper function so can be used for both seps

@@ -37,6 +37,7 @@ export interface ProcessingAnchorTransaction extends BaseTransaction {
amount_out_asset?: string;
amount_out: string;
amount_fee_asset?: string;
quote_id?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note sure why this was left off (I may have just missed it before)

@acharb acharb merged commit 064cf2c into release/1.3.0 Nov 29, 2023
2 checks passed
@acharb acharb deleted the acharb-sep6-polling branch November 29, 2023 17:54
async getTransactionsForAsset({
authToken,
assetCode,
account,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need an account here? Can't we get it from the auth token? I think in real implementations there are no use cases where account != SEP-10 account

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sep 6 requires it. If you don't give an account that matches the auth token account it gives a 403.

I'm not with my laptop right now but I can link to it on the sep 6 spec tonight

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm suggesting to omit it in the method parameter, and instead always pass account from the auth token to the anchor. (that's how anchor platform and polaris work, as they require authentication in /transaction endpoint, therefore account must always match authenticated account)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh I see. Ya I agree that's better. We have this ticket for tying the account to the auth token (right now it's just a string) https://stellarorg.atlassian.net/browse/WAL-1152?atlOrigin=eyJpIjoiYjMxYWY2NDRkY2IyNGE4MGE4MjlkNWM3NDFmNzVlMDQiLCJwIjoiaiJ9

I'll make a note on the ticket to change this as well to always pull from the auth token

@acharb acharb mentioned this pull request Dec 12, 2023
piyalbasu added a commit that referenced this pull request Jan 8, 2024
* add type aliases, and make sep24 example use env vars (#66)

* add type aliases

* make example use env vars

* Account setup and recovery using SEP-30 (#65)

* Add AccountRecover class

* Add Recovery class

* Add builderExtra param

* Fix importing

* Swallow 404 error on registerRecoveryServerSigners function

* Remove todo comment

* Use plural

* Add basic testing

* increase timeout

* Move private functions to the bottom

* Make AccountRecover abstract

* Rename type to WalletRecoveryServers

* add Customer / SEP-12 code (#71)

* add type aliases

* make example use env vars

* finish tests

* add types

* cleanup

* comment update

* use getCustomer

* jsdoc

* for binary data

* clean up

* cleanup

* use sep9BinaryInfo

* cleanup

* add gha and linting (#72)

* add type aliases

* make example use env vars

* finish tests

* add types

* cleanup

* add gha

* clean up all files

* add prettier lint

* sep12 stuff linting

* cleanup

* move to devdeps

* add jsdocs to all major functions (#73)

* add type aliases

* make example use env vars

* finish tests

* add types

* cleanup

* add gha

* clean up all files

* add prettier lint

* sep12 stuff linting

* cleanup

* add for all classes

* more comments

* more fixes

* more changes

* cleanup

* small fixes to Recovery code (#75)

* small fixes

* fix path payment test

* add sep6 get info (#76)

* add SEP6 deposit and withdrawal (#77)

* add sep6 deposit and withdrawal

* comment

* make better response

* remove wrapper type

* add exchange endpoints (#78)

* adding recovery integration tests (#74)

* first cut

* try workflow

* run daemon

* print logs

* restart on failure

* try this

* try

* try

* try

* try

* cross fingers

* fix

* works hopefully

* fix

* add back

* cleanup

* image versions and readme

* add watcher and polling to sep6 (#79)

* upgrade stellar-sdk to v11.0.0-beta.6 (#80)

* add authToken class (#81)

* add authToken class

* make account optional

* cleanup

* remove un-needed account

* adding some helper methods (#82)

* account merge and premade assets (#83)

* add sep38 info, price, and prices (#84)

* sep38 getinfo

* add prices info, cleanup

* move authToken to optional arg

* add jsdoc

* throw if both buy and sell amount given

* add sep38 quote (#85)

* sep38 getinfo

* add prices info, cleanup

* first cut

* move authToken to optional arg

* cleanup

* add jsdoc

* throw if both buy and sell amount given

* make context type right (#86)

* fix test

* bumping package version

---------

Co-authored-by: Alec Charbonneau <[email protected]>
Co-authored-by: Cássio Marcos Goulart <[email protected]>
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.

3 participants