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

AVM: incentive opcodes #5984

Merged
merged 133 commits into from
May 15, 2024
Merged

AVM: incentive opcodes #5984

merged 133 commits into from
May 15, 2024

Conversation

jannotti
Copy link
Contributor

Adds AVM access to things that make sense for smart contracts related to block incentives.

Make migrate use VoteID.IsEmpty()
Improve random accounts, especially for basics.Suspended
Validate proposed absents, and suspend them.
v40

Use SelectionID to detect when to zero out StateProofID

Simplify online account delta handling

Fix expiration test
@jannotti jannotti marked this pull request as ready for review April 23, 2024 19:09
Copy link
Member

@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.

Separate topic: do we want to make a new REST endpoint for voting account data? It seems a bit odd that there's a reason to expose this stuff for on-chain contracts which wouldn't also apply to off-chain users as well.

data/transactions/logic/fields.go Show resolved Hide resolved
daemon/algod/api/server/v2/dryrun.go Outdated Show resolved Hide resolved
cmd/tealdbg/localLedger.go Show resolved Hide resolved
ledger/eval_simple_test.go Show resolved Hide resolved
Copy link

codecov bot commented Apr 29, 2024

Codecov Report

Attention: Patch coverage is 33.10811% with 99 lines in your changes are missing coverage. Please review.

Project coverage is 56.11%. Comparing base (b259709) to head (9743a9f).
Report is 17 commits behind head on master.

Files Patch % Lines
ledger/eval/eval.go 3.44% 28 Missing ⚠️
cmd/tealdbg/localLedger.go 0.00% 18 Missing ⚠️
daemon/algod/api/server/v2/dryrun.go 0.00% 18 Missing ⚠️
data/transactions/logic/eval.go 68.42% 8 Missing and 4 partials ⚠️
ledger/eval/applications.go 0.00% 9 Missing ⚠️
data/transactions/logic/fields.go 72.72% 5 Missing and 1 partial ⚠️
ledger/tracker.go 0.00% 4 Missing ⚠️
data/transactions/logic/fields_string.go 50.00% 1 Missing and 1 partial ⚠️
ledger/eval/cow.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5984      +/-   ##
==========================================
- Coverage   56.13%   56.11%   -0.02%     
==========================================
  Files         482      481       -1     
  Lines       68027    68074      +47     
==========================================
+ Hits        38187    38203      +16     
- Misses      27243    27273      +30     
- Partials     2597     2598       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jannotti
Copy link
Contributor Author

Separate topic: do we want to make a new REST endpoint for voting account data? It seems a bit odd that there's a reason to expose this stuff for on-chain contracts which wouldn't also apply to off-chain users as well.

I guess so, because it's strange. On the other hand, I don't know if it's all that useful. I feel like we're adding these opcodes to fill a potential hole in the ability to make correct contracts that can do almost anything the in protocol incentive scheme can do. If you're off-chain, you have a lot of extra flexibility - For example you can record and use the old (balance round) information.

@jasonpaulos
Copy link
Member

I guess so, because it's strange. On the other hand, I don't know if it's all that useful. I feel like we're adding these opcodes to fill a potential hole in the ability to make correct contracts that can do almost anything the in protocol incentive scheme can do. If you're off-chain, you have a lot of extra flexibility - For example you can record and use the old (balance round) information.

Ok, that's convincing enough for now. At the very least, I don't think expanding the REST API should happen in this PR.

algorandskiy
algorandskiy previously approved these changes Apr 30, 2024
ledger/eval/eval.go Show resolved Hide resolved
ledger/eval/eval.go Outdated Show resolved Hide resolved
jasonpaulos
jasonpaulos previously approved these changes May 1, 2024
@pbennett
Copy link

pbennett commented May 1, 2024

Things I need for Reti at the moment (looks like 2 of the 4-5 are covered here)
✅ Current online stake
✅ Whether an account is incentive eligible (so I can know if extra fee is needed when going online)
❌ The 'fee' required when going online to make the account IncetiveEligible.
❌ The maximum algo amount for incentives
❌ The minimum amount for incentives would be nice as well (but not 'needed')

@jannotti jannotti dismissed stale reviews from jasonpaulos and algorandskiy via 64ff41b May 8, 2024 18:08
@jannotti
Copy link
Contributor Author

#5998

algorandskiy
algorandskiy previously approved these changes May 13, 2024
@jannotti jannotti requested a review from gmalouf May 14, 2024 18:29
ledger/eval/eval.go Outdated Show resolved Hide resolved
@gmalouf gmalouf merged commit 98fdd2a into algorand:master May 15, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants