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

Driver & Network Selection - new mechanism #3023

Closed
golmek opened this issue Jan 24, 2024 · 9 comments · Fixed by golemfactory/ya-client#156, #3031 or #3042
Closed

Driver & Network Selection - new mechanism #3023

golmek opened this issue Jan 24, 2024 · 9 comments · Fixed by golemfactory/ya-client#156, #3031 or #3042
Assignees

Comments

@golmek
Copy link
Contributor

golmek commented Jan 24, 2024

Context:

Currently configuration of payment platform (driver + network + token) is done by application. It has some benefits, however also introduces important problems:

  • When default driver is changed in Yagna, SDK still uses old driver
  • When default network is changed in Yagna, SDK still uses old default network
  • At least few times a year ETH testnet has smaller or bigger downtime or is overloaded

Consequently, we want to change our approach and get rid off precise specification of payment platform on allocation creation (flow between SDK → API → Yagna requestor).

Solution:

Payment Platform:

Payment Platform consists of 3 values: driver, network, token

We want:

  • driver - to have default value of erc20 (underneath new payment driver will be hidden - Rename erc20next to erc20 #3019)
  • network - to have default value of holesky for tglm and polygon for glm
  • token → For instance for default values of erc20 or/and holesky it should return tglm. For values of erc20 or/and polygon it should return glm

Allocation Creation

SDK will create allocation in following fashion:

Example A: default driver, netwrok and token:
Request:

{
    "totalAmount": "10",
    "makeDeposit": false
}

Alternative Request:

{
    "paymentPlatform": {},
    "totalAmount": "10"
    "makeDeposit": false
}

Response:

{
    "allocationId": "520008f2-ffb5-427e-b9ec-17173520b78b",
    "address": "0x889ff52ece3d5368051f4f8216650a7843f8926b",
    "paymentPlatform": "erc20-holesky-tglm",
    "totalAmount": "10",
    "spentAmount": "0",
    "remainingAmount": "0",
    "timestamp": "2024-01-19T12:29:35.278Z",
    "timeout": "2034-01-19T15:29:34.535594Z",
    "makeDeposit": false
}

Example B: default driver on polygon

{
    "paymentPlatform": {
        "network": "polygon",  // Optional
    },
    "totalAmount": "10"
    "makeDeposit": false
}

Result:

{
    "allocationId": "520008f2-ffb5-427e-b9ec-17173520b78b",
    "address": "0x889ff52ece3d5368051f4f8216650a7843f8926b",
    "paymentPlatform": "erc20-polygon-glm",
    "totalAmount": "10",
    "spentAmount": "0",
    "remainingAmount": "0",
    "timestamp": "2024-01-19T12:29:35.278Z",
    "timeout": "2034-01-19T15:29:34.535594Z",
    "makeDeposit": false
}

Example C: full platform specified

Request (backward compatible):

{
    "paymentPlatform": "erc20-polygon-glm",
    "totalAmount": "10"
    "makeDeposit": false
}

Alternative Request:

{
    "paymentPlatform": {
        "driver": "erc20",            // Optional
	"network": "polygon",    // Optional
	"token": "glm"               // Optional 
    },
    "totalAmount": "10"
    "makeDeposit": false
}

Result

{
    "allocationId": "520008f2-ffb5-427e-b9ec-17173520b78b",
    "address": "0x889ff52ece3d5368051f4f8216650a7843f8926b",
    "paymentPlatform": "erc20-polygon-glm",
    "totalAmount": "10",
    "spentAmount": "0",
    "remainingAmount": "0",
    "timestamp": "2024-01-19T12:29:35.278Z",
    "timeout": "2034-01-19T15:29:34.535594Z",
    "makeDeposit": false
}

Example D: default driver and network for glm token:

{
    "paymentPlatform": {
	"token": "glm"           // Optional 
    },
    "totalAmount": "10"
    "makeDeposit": false
}

Result:

{
    "allocationId": "520008f2-ffb5-427e-b9ec-17173520b78b",
    "address": "0x889ff52ece3d5368051f4f8216650a7843f8926b",
    "paymentPlatform": "erc20-polygon-glm",
    "totalAmount": "10",
    "spentAmount": "0",
    "remainingAmount": "0",
    "timestamp": "2024-01-19T12:29:35.278Z",
    "timeout": "2034-01-19T15:29:34.535594Z",
    "makeDeposit": false
}

Acceptance Criteria:

  • golemjs / actions/ -> release test

Additional Information:

@golmek golmek changed the title Network Selection - new approach Driver & Network Selection - new mechanism Jan 24, 2024
@golmek
Copy link
Contributor Author

golmek commented Jan 24, 2024

@grisha87 @shadeofblue @nieznanysprawiciel - above ticket is a conclusion of our Friday discussion regarding default network/driver selection on Allocation creation. Please verify it that looks OK from your perspective (SDK - JS + Python)

@golmek
Copy link
Contributor Author

golmek commented Jan 25, 2024

Blocked till feedback is gathered.

@grisha87
Copy link
Contributor

To the AC I would add that the pre-release containing the fix should successfully pass the tests on master and beta for this pipeline:

https://github.com/golemfactory/golem-js/actions/workflows/release.yml

How to run it:

  1. Open the release pipeline
  2. Manually trigger the pipeline for master branch and put the pre-release information into the dialog
  3. Repeat the test for beta branch

Success criteria

  • Both test runs finish successfully

@golmek
Copy link
Contributor Author

golmek commented Jan 25, 2024

Suggestion:

  • New Endpoint (only objects in Requests and responses)
  • ready for allowance

@shadeofblue
Copy link
Contributor

re: example D -> my expectation would be for yagna to give me the "recommended" platform that will satisfy the requirement that I have given in my request -> so, in other words, my interpretation of the request is: give me the "default" of those platforms that support the glm token ...

or, in yet another set of words, give me the default mainnet platform...

such as it implemented, it will prevent forwards-compatibility of allowing the SDK to select the default mainnet platform... e.g. if we wanted to switch from polygon to some other blockchain or from erc20 to some other driver...

@shadeofblue
Copy link
Contributor

To the AC I would add that the pre-release containing the fix should successfully pass the tests on master and beta for this pipeline:

https://github.com/golemfactory/golem-js/actions/workflows/release.yml

How to run it:

[cut]

Success criteria

* Both test runs finish successfully

uh, it would be good to conduct also tests with other requestor products as JS SDK is very far from the only consumer here ;p

@scx1332
Copy link
Collaborator

scx1332 commented Feb 1, 2024

Added pull request
golemfactory/ya-client#156

@golmek
Copy link
Contributor Author

golmek commented Feb 5, 2024

back to development on request of @nieznanysprawiciel - 1MD

@golmek golmek reopened this Feb 5, 2024
@scx1332 scx1332 linked a pull request Feb 5, 2024 that will close this issue
@golmek
Copy link
Contributor Author

golmek commented Feb 7, 2024

RC 14 has those changes

@golmek golmek closed this as completed Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants