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.

Sign up for GitHub

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

Jump to bottom

Processes pay for executions #1699

Merged
merged 7 commits into from
Mar 13, 2020
Merged

Processes pay for executions #1699

merged 7 commits into from
Mar 13, 2020

Conversation

krhubert
Copy link
Contributor

@krhubert krhubert commented Mar 3, 2020 •
edited by NicolasMahe
Loading

dependant on #1688

close #1698

@krhubert krhubert added the enhancement New feature or request label Mar 3, 2020
@krhubert krhubert added this to the next milestone Mar 3, 2020
@krhubert krhubert self-assigned this Mar 3, 2020
@krhubert krhubert mentioned this pull request Mar 5, 2020
Closed
@NicolasMahe NicolasMahe added the release:add Pull requests that add something label Mar 9, 2020
NicolasMahe
NicolasMahe reviewed Mar 9, 2020
View reviewed changes
Copy link
Member

@NicolasMahe NicolasMahe left a comment

Choose a reason for hiding this comment

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

looking good 👍
Can you add a test that triggers a process without balance?

@antho1404
Copy link
Member

Let's merge it when we are sure that we can credit a process as well.
Right now we cannot credit a process so every process will fail because their balances will be empty.
We need to make sure that this feature will not break the processes. Let's merge when we have either:

This is blocking, even this PR is working, it needs to be merged only when the client is updated otherwise all processes will be broken.

@NicolasMahe NicolasMahe mentioned this pull request Mar 11, 2020
Merged
4 tasks
@antho1404 antho1404 mentioned this pull request Mar 12, 2020
Merged
@NicolasMahe
Copy link
Member

NicolasMahe commented Mar 13, 2020 •
edited
Loading

TODO:

  • take coins from signer when create a process (hardcode everything)

@NicolasMahe NicolasMahe assigned NicolasMahe and unassigned krhubert Mar 13, 2020
@NicolasMahe NicolasMahe changed the title Get coins from execution process if attached Get coins from execution's process if attached Mar 13, 2020
antho1404
antho1404 reviewed Mar 13, 2020
View reviewed changes
e2e/orchestrator_balance_withdraw_test.go
"github.com/tendermint/tendermint/crypto"
)

func testOrchestratorProcessBalanceWithdraw(executionStream pb.Execution_StreamClient, instanceHash hash.Hash) func(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

This test is not a withdraw test it more that the process actually paid correctly for the execution

@NicolasMahe NicolasMahe merged commit 970eb34 into dev Mar 13, 2020
@NicolasMahe NicolasMahe deleted the feature/process-coins branch March 13, 2020 09:28
@NicolasMahe NicolasMahe changed the title Get coins from execution's process if attached Processes pay for executions Mar 17, 2020
@NicolasMahe NicolasMahe mentioned this pull request Mar 17, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antho1404 antho1404 antho1404 left review comments

@NicolasMahe NicolasMahe NicolasMahe approved these changes

Assignees

@NicolasMahe NicolasMahe

Labels
enhancement New feature or request release:add Pull requests that add something
Projects
None yet
Milestone
v0.20.0
Development

Successfully merging this pull request may close these issues.

Add balance system on processes
3 participants
@krhubert @antho1404 @NicolasMahe