Feat/qpay - #977
Open
double-k-3033 wants to merge 10 commits into
Open
Conversation
Adds operator/recovery-gated promo pricing (SetPromoRate/RemovePromoRate/ChangeOperator/GetPromoRate), ported and adapted from profitphil/qubic-x402 PR #3 after a security review of the access-control and rate-bound logic. Discounted rates are clamped to [0.25%, 0.75%] and can never exceed the standard fee. Adds matching GoogleTest coverage for the new procedures.
Adds affiliateRegistrarId-gated referral links (SetAffiliate/RemoveAffiliate/ChangeAffiliateRegistrar/GetAffiliate), ported and adapted from profitphil/qubic-x402 PR #3 after a security review. A referrer earns 5% of a referred seller's fee for 52 epochs (~1 year), taken out of the fee (never the seller's net or buyer's payment). Self-referral is blocked, first-attribution-wins, and the link expires/purges the same way receipts do. affiliateRegistrarId is a separate admin role from operatorId so a compromised key only ever touches one surface; recoveryId can reassign either. Adds matching GoogleTest coverage.
double-k-3033
force-pushed
the
feat/Qpay
branch
from
August 14, 2026 09:53
88d5b0f to
dd83c14
Compare
Contributor
|
Please let us know when this PR is ready for review. Also please add a description of the PR before that. |
Contributor
Author
https://github.com/profitphil/Proposal-deploy-QPAY-smart-contract |
Contributor
Thanks, short notice the link at the end goes 404: https://github.com/profitphil/qubic-x402 |
Contributor
Author
invited you. plz check again |
Contributor
|
Sadly the test suits runs into some errors on my machine: Please double check on your side. |
Three ContractQPayhub tests failed due to fixture bugs, not contract bugs;
src/contracts/QPayhub.h is unchanged.
EndEpochDistributesFeePoolAboveReserveToSharesAndTokenHolders funded the
QPAY issuer with 10,000,000 QU while QX charges 1,000,000,000 QU to issue
an asset, so IssueAsset refunded and issued nothing, leaving no token
holders for the 90% split. Fund from qxFees instead of a hardcoded amount.
NotifyQuUsdPriceReply{Valid,Invalid}ResolvedReply seeded
broadcastedComputors from computorPublicKeys, which holds only one entry
(sized by computorSeeds). Indices 1..675 read out of bounds, so nearly
every computor mapped to the same computorIndex(), all 676 commits
collapsed onto one slot, quorum (451) was never reached and the query
stayed PENDING with no reply to fetch. Use distinct synthetic keys and
init the engine from them, matching OracleEngineTest.
All 53 ContractQPayhub tests pass.
Contributor
Author
fixed it |
fnordspace
approved these changes
Aug 19, 2026
fnordspace
left a comment
fnordspace
left a comment
Contributor
There was a problem hiding this comment.
From a formal point of view the contract seems fine for me. Please note that we do not review the logic of the contract as part of the core team.
I annotated two things that I think are worth double checking, in particular also because I did not find test results for the oracle operation in the provided repo. (Maybe just did not find it or I was on the wrong branch).
Also I did not see any qpi.burn() in the contract, maybe I missed the note about the economic model but at the moment the contract reserve is not refilled and might run out after a moment.
I would highly suggest to have a proper external code review besides AI and a Security Audit as this is a payment system!
| // of the file - the same pattern TestExampleC.h uses. | ||
| using namespace Ch; | ||
| locals.query.currency1 = id(Q, U, B, I, C); | ||
| locals.query.currency2 = id(U, S, D, T); |
Contributor
There was a problem hiding this comment.
Not sure if that works reliably. It would be saver to change to id(U, S, D, T, null);. See TestExampleC.h
| // rejection path. | ||
| PUBLIC_PROCEDURE_WITH_LOCALS(SubscribeToPriceFeed) | ||
| { | ||
| if (state.get().priceOracleSubscriptionId >= 0) |
Contributor
There was a problem hiding this comment.
I suspect this will work for one epoch and then you can't re-subscribe with this guard present as priceOracleSubscriptionId = 0 after initial subscription and is not reset during epoch change.
Both issues were flagged by fnordspace in review on qubic#977. The USDT side of the query was never USDT. id()'s character constructor declares c0..c4 without defaults (m256.h), so the four-argument id(U, S, D, T) is not viable for it and overload resolution silently falls through to id(uint64, uint64, uint64, uint64), storing the four ASCII codes as separate 64-bit limbs instead of packing "USDT" into the first four bytes. The compiler accepts this without a warning. Pad explicitly with null, as TestExampleC.h does. The price feed also died permanently after one epoch. The node drops every oracle subscription during the epoch transition (qubic.cpp beginEpoch() -> oracleEngine.beginEpoch() -> reset()) and runs each contract's BEGIN_EPOCH only afterwards, but priceOracleSubscriptionId was written only by INITIALIZE and by a successful subscribe. The stale id kept failing the >= 0 guard in SubscribeToPriceFeed, so no renewal could ever succeed. Add a BEGIN_EPOCH that clears it back to -1; renewal stays permissionless. Two tests, both confirmed failing before the fix. SubscribeToPriceFeedUsesQubicUsdtCurrencyPair exploits the engine's query dedup (timestamp excluded): a probe subscription from a different contract carrying the intended QUBIC/USDT query must land on the same subscription id - pre-fix it got a second one. BeginEpochClears StaleSubscriptionSoFeedCanBeRenewed drives a realistic rollover and re-subscribes - pre-fix that returned ERR_ALREADY_SUBSCRIBED. The existing already-subscribed test still passes, so the guard continues to reject duplicate subscribes within an epoch. All 55 ContractQPayhub tests pass.
Changes the END_EPOCH split from 10/90 to 10/1/89 - shareholders keep their 10%, the 1% burn comes out of the token-holder slice. Addresses fnordspace's review note on qubic#977 that the contract had no qpi.burn() anywhere and its reserve was never refilled, so it would run out. qpi.burn() is not a plain supply sink: it takes QU out of the contract's spectrum balance and credits the contract fee reserve of the index passed to it, defaulting to the calling contract (qpi_spectrum_impl.h: burn -> decreaseEnergy + addToContractFeeReserve). Burning for QPayhub itself makes the contract fund its own execution out of its own revenue. The burn runs before either payout and is clamped to the live balance. It is the slice that keeps the contract runnable, so it must not be the one squeezed out when the balance runs short. It counts into `distributed` because the QU really does leave the contract balance, so feePool has to shrink by it like any other outflow, and it is only committed if qpi.burn() reports success. QPAYHUB_DIVIDEND_BURN_PERMILLE is asserted at compile time to leave a non-negative remainder for token holders. totalBurned on the state and burnPermille/totalBurned on GetInfo are appended at the end of their structs, following the convention those structs already document for keeping existing field offsets stable. EndEpochDistributesFeePoolAboveReserveToSharesAndTokenHolders updated to the new numbers: on 676,000 distributable that is 67,600 / 6,760 / 601,640, summing exactly to 676,000 with nothing stranded. Two new tests: EndEpochBurnsOnePercentIntoOwnExecutionFeeReserve asserts via getContractFeeReserve() that the reserve grew by exactly the burn and the balance fell by it, deliberately with no QPAY token issued so the burn is shown not to depend on there being token holders to pay; EndEpochBurnsNothingWhenFeePoolAtOrBelowReserve guards against the burn eating into the reserve it is meant to protect. All 57 ContractQPayhub tests pass.
Contributor
Author
|
@fnordspace I added burn. could you check this? |
Contributor
|
Looks ok to me. |
|
can you merge the PR in qubic/proposals/sc so I can send to Comps. thx |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.