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

Economic Protocol scenario 3 - "contract earns fee" #1735

Merged
merged 27 commits into from
Jun 4, 2024

Conversation

miloszm
Copy link
Contributor

@miloszm miloszm commented May 14, 2024

This PR implements Economic Protocol scenario 3 - "contract earns fee"
It contains new test contract Charlie, changes to the Transfer Contract,
relevant changes in the block generator, and tests for scenarios related to the EP scenario 3
(both wallet based and transaction based).

Implements issues:
#1603
#1604
#1630

rusk/src/lib/chain/rusk.rs Outdated Show resolved Hide resolved
contracts/subsidy-types/Cargo.toml Outdated Show resolved Hide resolved
contracts/transfer/Cargo.toml Outdated Show resolved Hide resolved
contracts/transfer/tests/common/utils.rs Outdated Show resolved Hide resolved
contracts/transfer/tests/common/utils.rs Outdated Show resolved Hide resolved
Copy link
Member

@ureeves ureeves left a comment

Choose a reason for hiding this comment

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

Apart from the things mentioned below, there's one more thing I'd like to at least have discussed - what happens when the contract charges a fee and the amount goes over the gas limit?

contracts/transfer/src/state.rs Outdated Show resolved Hide resolved
contracts/transfer/src/state.rs Outdated Show resolved Hide resolved
contracts/transfer/src/state.rs Outdated Show resolved Hide resolved
rusk/src/lib/chain/rusk.rs Outdated Show resolved Hide resolved
contracts/transfer/tests/scenario3.rs Outdated Show resolved Hide resolved
contracts/transfer/src/state.rs Outdated Show resolved Hide resolved
rusk/src/lib/chain/rusk.rs Outdated Show resolved Hide resolved
contracts/transfer/src/state.rs Outdated Show resolved Hide resolved
contracts/transfer/src/state.rs Outdated Show resolved Hide resolved
@miloszm
Copy link
Contributor Author

miloszm commented May 16, 2024

re: "these should be two separate fields and have Rusk decide payings and refunds" (Federico) and " it can always be used by rusk itself to compute the refund" (Ed) - I think there is a misunderstanding here. There is no refund to be decided, refund has already been paid, and it is on contract's account by the time it reaches this point. So I think it is an incorrect idea about Rusk to be deciding something. We can add an extra field merely as information - this can be done and maybe makes life easier, as otherwise this info is available via events anyway.

@miloszm
Copy link
Contributor Author

miloszm commented May 16, 2024

Re: what happens when the contract charges a fee and the amount goes over the gas limit? - if charge > cost, the charge is not applied, and the behaviour is just "normal", i.e., user pays the normal cost, and the charging contract does not earn anything.

@miloszm miloszm force-pushed the issue-1630-contract-earns-fee branch 2 times, most recently from a9c4f04 to 2a82b72 Compare May 16, 2024 11:52
contracts/transfer/src/state.rs Outdated Show resolved Hide resolved
rusk/src/lib/chain/rusk.rs Outdated Show resolved Hide resolved
rusk/src/lib/chain/rusk.rs Outdated Show resolved Hide resolved
contracts/subsidy-types/Cargo.toml Outdated Show resolved Hide resolved
contracts/subsidy-types/src/lib.rs Outdated Show resolved Hide resolved
contracts/subsidy-types/README.md Outdated Show resolved Hide resolved
node-data/src/ledger.rs Outdated Show resolved Hide resolved
rusk-recovery/src/state.rs Outdated Show resolved Hide resolved
@miloszm miloszm force-pushed the issue-1630-contract-earns-fee branch 12 times, most recently from b550f6f to ed5c807 Compare May 20, 2024 13:46
@miloszm miloszm force-pushed the issue-1630-contract-earns-fee branch from 27c56e7 to 30a4a95 Compare May 22, 2024 08:05
@herr-seppia herr-seppia self-requested a review May 22, 2024 09:08
@herr-seppia herr-seppia dismissed their stale review June 4, 2024 09:03

No need for now to have 2 different contract balances

Copy link
Member

@ureeves ureeves left a comment

Choose a reason for hiding this comment

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

LGTM

@miloszm miloszm merged commit b19ab25 into master Jun 4, 2024
8 checks passed
@miloszm miloszm deleted the issue-1630-contract-earns-fee branch June 4, 2024 20:08
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.

5 participants