-
Notifications
You must be signed in to change notification settings - Fork 151
IPO QThirtyFour #684
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
base: develop
Are you sure you want to change the base?
IPO QThirtyFour #684
Conversation
# Conflicts: # src/Qubic.vcxproj # src/contract_core/contract_def.h
…reserve and don’t consume reseed budget with tier top-ups on k4 rounds - test/contract_qtf.cpp: expose private/protected internals for unit tests, add exact-match k2/k3 ticket generators (unique), fund jackpot balance in k4 test, and force FR off in baseline k2/k3 revenue-split test
# Conflicts: # src/contract_core/contract_def.h # src/contracts/RandomLottery.h # test/test.vcxproj # test/test.vcxproj.filters
…eserve top-ups; correct schedule bitmask in specx
|
|
||
| // Number of available smart contracts in the QRP contract. | ||
| static constexpr uint16 QRP_AVAILABLE_SC_NUM = 128; | ||
| static constexpr uint64 QRP_QTF_INDEX = 21; |
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.
Just a reminder that this index needs to be adapted when merging. There are some more SCs in the pipeline so indices are currently unclear. It becomes especially tricky when we have multiple proposals in one epoch because indices then depend on proposal outcomes.
Maybe it would be best to define this as QRP_CONTRACT_INDEX + 1. This should always be correct assuming QRP and QTF can only be accepted together (=single proposal)?
| return; | ||
| } | ||
|
|
||
| state.availableSmartContracts.remove(id(input.scIndex, 0, 0, 0)); |
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.
this HashMap needs a call to cleanup or cleanupIfNeeded because it's calling remove (see https://github.com/qubic/core/blob/main/doc/contracts.md#container-types)
| { | ||
| // Procedures | ||
| REGISTER_USER_PROCEDURE(GetReserve, 1); | ||
| REGISTER_USER_PROCEDURE(AddAvailableSC, 2); |
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.
Up to you but I suggest to rename *AvailableSC to *AllowedSC. Available reserve makes sense to me but the SCs are either allowed or not.
| } | ||
| } | ||
|
|
||
| public: |
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.
this is already public (or are you repeating it for easier readability?)
|
|
||
| #define _ALLOW_KEYWORD_MACROS 1 | ||
|
|
||
| #define private protected |
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 in the QDuel test, why is this needed?
Proposal:
QTF
QRP