-
Notifications
You must be signed in to change notification settings - Fork 37
fix(contract): drop the deposit for submit_participant_info
#3940
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
Merged
Merged
Changes from 17 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
3790000
fix(contract): charge attestation storage by actual delta
pbeza c0886f0
fix(contract): make attestation deposit const pub, address review
pbeza d35db9b
Merge remote-tracking branch 'origin/main' into 3857-attestation-stor…
pbeza ba694a9
fix(contract): regenerate ABI snapshot, use fail-loud balance math in…
pbeza 6b4cbeb
test(contract): parametrize worst-case storage test, tighten attestat…
pbeza c471323
Merge remote-tracking branch 'origin/main' into 3857-attestation-stor…
pbeza c67cea5
fix(contract): charge attestation storage only for new entries or non…
pbeza e7128f2
Merge remote-tracking branch 'origin/main' into 3857-attestation-stor…
pbeza b1ab8eb
fix(contract): charge attestation storage by measured delta; fund e2e…
pbeza c8d62f9
Merge remote-tracking branch 'origin/main' into 3857-attestation-stor…
pbeza 3efa6e4
fix(contract): fund attestation storage from the contract balance
pbeza 8af54e5
Merge remote-tracking branch 'origin/main' into 3857-attestation-stor…
pbeza 7b7444c
refactor(contract): drop now-vestigial attestation-store flush
pbeza e693f1c
test(contract): drop dead deposit context, fix stale refund naming
pbeza 0ca1cc9
test(contract): restore MpcContractHandle sandbox helper, add given/w…
pbeza 29e7520
docs: correct attestation-verifier deposit note to contract-funded st…
pbeza c88ff3e
Merge remote-tracking branch 'origin/main' into 3857-attestation-stor…
pbeza afacfef
refactor(node): drop deposit threading from the tx path
pbeza 296539d
refactor(contract): reuse storage-cost idiom for attestation entry cap
pbeza 5a164a8
test(contract): drop tautological entry-cost test, inline single-use …
pbeza e845e90
refactor(contract): measure real storage delta for the attestation en…
pbeza 158e8e4
revert(contract): drop the runtime attestation entry-size cap
barakeinav1 74ea6a0
test(contract): pin the attestation entry size, tighten the cost ceiling
barakeinav1 a80955c
Merge remote-tracking branch 'origin/main' into 3857-attestation-stor…
barakeinav1 1428b4d
docs: mark the superseded yield-resume sections with TODO(#3825)
barakeinav1 d12a0c8
docs: submit_participant_info is node-submission only
barakeinav1 aae164f
docs: drop the stale deposit guidance, widen the superseded-design ba…
barakeinav1 d8c3579
test(contract): name the entry-size tests after their subject
barakeinav1 26c343b
test(contract): pin that submit_participant_info rejects an attached …
barakeinav1 bf3fe13
docs: use the required TODO(#NNNN): format in the status banner
barakeinav1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
It’s effectively the same as the pre-#3714 version of the contract. IIUC, v3.13.0 charged the measured delta, but
add_participantnever flushed the storage, so the delta was always read as0and newMockentries were effectively free. The drain existed there too.I’m not quite sure what a better funding model for the new attestations would be, given that the nodes’ function-call access keys can’t pay for storage.
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.
Agreed this isn't a regression and shouldn't block. One correction to the framing though, since #3972 is being written against it: this needs no TEE at all.
MockAttestation::ValidreturnsOk(())unconditionally fromverify_constraints,submit_participant_infohas no participant gate (justassert_caller_is_signer, which only rules out cross-contract calls), and entries are keyed bytls_public_keywith no per-account limit — so a single ordinary account can mint unbounded entries with no attestation hardware, for gas only.They're also permanent:
clean_invalid_attestationsremoves only entries that fail re-verification, andMock::Validre-verifies as valid forever with no expiry field to age out. So the "bounded and self-healing, reclaimed byclean_invalid_attestations" argument holds forDstack(which does expire and does get swept) but not for the cheap path.So the Slack framing of "an arbitrary entity with a single TEE can slowly drain" is understating it. Worth #3972 pricing out the mock path explicitly — and gating
Attestation::Mockbehind a feature flag would remove the cheap variant independently of the deposit work.