-
Notifications
You must be signed in to change notification settings - Fork 32
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
Changes for block incentives and v11 opcodes #106
Conversation
51a338d
to
1781094
Compare
1781094
to
4039875
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All understood and clear to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The only comments are about: missing FeeSink behavior in case of insufficient funds for the payout, a better specification of the units used in the descriptions of some fields/opcodes (as-in: microlgos
should be used instead of algos
), and some minor typos.
@@ -470,6 +470,11 @@ Fields | |||
| 15 | AssetCreateMinBalance | uint64 | v10 | The additional minimum balance required to create (and opt-in to) an asset. | | |||
| 16 | AssetOptInMinBalance | uint64 | v10 | The additional minimum balance required to opt-in to an asset. | | |||
| 17 | GenesisHash | [32]byte | v10 | The Genesis Hash for the network. | | |||
| 18 | PayoutsEnabled | bool | v11 | Whether block proposal payouts are enabled. | | |||
| 19 | PayoutsGoOnlineFee | uint64 | v11 | The fee required in a keyreg transaction to make an account incentive eligible. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we specify this number is in microalgos
?
| 21 | PayoutsMinBalance | uint64 | v11 | The minimum algo balance an account must have in the agreement round to receive block payouts in the proposal round. | | ||
| 22 | PayoutsMaxBalance | uint64 | v11 | The maximum algo balance an account can have in the agreement round to receive block payouts in the proposal round. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before, I think it would be better to point out the min/max are expressed in microalgos (unless I'm missing something).
Participation updates contains a two list of addressess of accounts | ||
for which changes are made to their particpation status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typos: addressess
(LOC 596) and particpation
(LOC 597)
|
||
The second contains accounts that have been deemed to be | ||
_suspended_. An account is said to be suspended according to the rules | ||
specified above for _suspended particpation accounts_ list. Once |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: particpation
(LOC 607)
This adds explanations for block incentive payouts, heartbeat transactions, logicsig pooling and AVM v11 opcodes.