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.

Sign up for GitHub

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

Jump to bottom

Wrap witness-taking consensus programs in pre-loaded data audit snippets #190

Open
10 of 14 tasks
Sword-Smith opened this issue Sep 26, 2024 · 0 comments
Open
10 of 14 tasks

Wrap witness-taking consensus programs in pre-loaded data audit snippets #190

Sword-Smith opened this issue Sep 26, 2024 · 0 comments
Assignees
@jan-ferdinand @Sword-Smith

Comments

@Sword-Smith
Copy link
Member

Sword-Smith commented Sep 26, 2024 •
edited
Loading

In order to verify that the field-size indicators are not maliciously set, we should audit the pre-loaded witnesses. Abstractions for this exist in the VerifyNdSiIntegrity in tasm-lib and in the AuditVmEndState in neptune-core. These checks have already been added to the update consensus program, so you can just pattern match on that.

Don't do this for the STARK verifier, as it already has similar checks built in for the Proof/vm_proof_iter pair.

Add to these snippets

  • transaction::RemovalRecordsIntegrity (only added at init because expensive)
  • transaction::CollectLockScripts (only added at init because expensive)
  • transaction::KernelToOutputs
  • transaction::CollectTypeScripts
  • type_scripts::NativeCurrency
  • type_scripts::TimeLock
  • transaction::SingleProof
    • Collection case
    • Update case
    • Merger case
    • IntegralMempool case (program doesn't exist yet)
  • transaction::update
  • transaction::merge
  • All block-consensus programs
Sword-Smith added a commit that referenced this issue Sep 26, 2024
@Sword-Smith
removal_records_integrity: audit preloaded data
44eb9cd 
Add audit of preloaded data, without auditing at the end. This is done
this way because this check is unfortunately very expensive for the
removal records which contains a `ChunkDictionary` field which is a list
of variable-sized elements.

This check adds ~25% to the cost of this program, and the cost scales
linearly with the number of inputs.

A profile reveals the cost of this check, for both 2 and for inputs.

Cf. #190.
Sword-Smith added a commit that referenced this issue Sep 26, 2024
@Sword-Smith
collect_lock_scripts: audit preloaded witness
f843a48 
Benchmarks and profiles reveal that this is surprisingly expensive. So
we only do this check at program init. This program loops over
variable-sized elements *without* any range checks, so I consider this
commit essential for soundness.

Cf. #190.
Sword-Smith added a commit that referenced this issue Sep 26, 2024
@Sword-Smith
removal_records_integrity: audit preloaded data
0bf9cc6 
Add audit of preloaded data, without auditing at the end. This is done
this way because this check is unfortunately very expensive for the
removal records which contains a `ChunkDictionary` field which is a list
of variable-sized elements.

This check adds ~25% to the cost of this program, and the cost scales
linearly with the number of inputs.

A profile reveals the cost of this check, for both 2 and for inputs.

Cf. #190.
Sword-Smith added a commit that referenced this issue Sep 26, 2024
@Sword-Smith
collect_lock_scripts: audit preloaded witness
e959d61 
Benchmarks and profiles reveal that this is surprisingly expensive. So
we only do this check at program init. This program loops over
variable-sized elements *without* any range checks, so I consider this
commit essential for soundness.

Cf. #190.
aszepieniec pushed a commit that referenced this issue Sep 30, 2024
@Sword-Smith @aszepieniec
removal_records_integrity: audit preloaded data
54be9c2 
Add audit of preloaded data, without auditing at the end. This is done
this way because this check is unfortunately very expensive for the
removal records which contains a `ChunkDictionary` field which is a list
of variable-sized elements.

This check adds ~25% to the cost of this program, and the cost scales
linearly with the number of inputs.

A profile reveals the cost of this check, for both 2 and for inputs.

Cf. #190.
aszepieniec pushed a commit that referenced this issue Sep 30, 2024
@Sword-Smith @aszepieniec
collect_lock_scripts: audit preloaded witness
cadd64b 
Benchmarks and profiles reveal that this is surprisingly expensive. So
we only do this check at program init. This program loops over
variable-sized elements *without* any range checks, so I consider this
commit essential for soundness.

Cf. #190.
Sword-Smith added a commit that referenced this issue Sep 30, 2024
@Sword-Smith
collect_lock_scripts: audit preloaded witness
6c129f8 
Benchmarks and profiles reveal that this is surprisingly expensive. So
we only do this check at program init. This program loops over
variable-sized elements *without* any range checks, so I consider this
commit essential for soundness.

Cf. #190.
Sword-Smith added a commit that referenced this issue Sep 30, 2024
@Sword-Smith
collect_type_scripts: Audit preloaded data
3562b6b 
Again another fairly expensive check. Adds ~50 % to the processor
table's row count.

Cf. #190.
Sword-Smith added a commit that referenced this issue Sep 30, 2024
@Sword-Smith
kernel_to_outputs: Audit preloaded data
c446e7e 
Adds ~30 % to the processor table row count. Adds a specialed structure
for the memory-part of the witness data, as the row-count increase was
over 1000 % when the original witness-data was audited.

Cf. #190.
Sword-Smith added a commit that referenced this issue Oct 30, 2024
@Sword-Smith
tx-consensus: Audit preloaded data of type scripts
081d8a2 
Audit preloaded data of TimeLock and NativeCurrency.

Cf. #190.
Sword-Smith added a commit that referenced this issue Oct 30, 2024
@Sword-Smith
SingleProof: Audit preloaded witness data
ebdb760 
We cannot audit the witness data directly, as it's an enum and enums are
not supported by TasmObject, we audit the data associated with each of
the three variants that the witness can take.

Cf. #190.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jan-ferdinand jan-ferdinand

@Sword-Smith Sword-Smith

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jan-ferdinand @Sword-Smith