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.

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

[DRAFT][WIP] Code cleanup & docs during audit process #40

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2aeabff
Rough WIP - adding some notes related to kicking off the audit.
Olshansk Feb 8, 2024
b876799
WIP - Going through the extension node documentation
Olshansk Feb 8, 2024
d83d374
WIP - Moved extension node into its own file and removed the resolver…
Olshansk Feb 8, 2024
3cb019e
Split all the node types into different files
Olshansk Feb 8, 2024
562d7fa
Remove the unnecessary resolve helper and rename default value helpers
Olshansk Feb 8, 2024
04853f7
WIP - adding comments to different parts of extension_node.go
Olshansk Feb 8, 2024
6b368c0
Remove the need for the sum workaround function
Olshansk Feb 9, 2024
89f9ceb
WIP - refactoring node hasher's and encoders
Olshansk Feb 9, 2024
1857496
Renamed a few things and converted the examples to proper tests
Olshansk Feb 9, 2024
340d060
Added e2e_review.md and started going through the _node.go files
Olshansk Feb 10, 2024
ae20eab
A lot of things in flux but finished commenting node_encoders
Olshansk Feb 10, 2024
bff8a76
Add prefixLen in node encoders
Olshansk Feb 10, 2024
cd24a78
Interim checkpoint commit - modified lots of code and all tests pass …
Olshansk Feb 11, 2024
03d85ea
Improved how placeholder values are maintained and how the node is re…
Olshansk Feb 11, 2024
ee67740
Moved TrieSpec functions from utils into receiver functions
Olshansk Feb 11, 2024
c263cd7
Moved trie spec into its own file
Olshansk Feb 12, 2024
5e7ed76
chore: add context on hashing algorithms used by the trie
Mar 19, 2024
9e9d9b3
chore: add context on external kvstore writeability
Mar 19, 2024
4831b52
feat: consolidate ClosestProof verification and remove the NilPathHas…
Mar 19, 2024
46dc5c5
feat: reorganise extension node insertion to use separate pointers fo…
Mar 19, 2024
3862345
Old changes
Olshansk Mar 21, 2024
3981639
Merge branch 'main' into audit-remediations
h5law Apr 2, 2024
5199969
Review submittd PR
Olshansk Apr 8, 2024
3fe0d31
Merge with audit-remediations
Olshansk Apr 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@

# Ignore Goland and JetBrains IDE files
.idea/

# Ignore vscode files
.vscode
2 changes: 1 addition & 1 deletion bulk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func bulkOperations(t *testing.T, operations int, insert int, update int, delete
if err != nil && err != ErrKeyNotFound {
t.Fatalf("error: %v", err)
}
kv[ki].val = defaultValue
kv[ki].val = defaultEmptyValue
}
}

Expand Down
91 changes: 91 additions & 0 deletions docs/audit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# Audit

- [Audit](#audit)

## Pre-Audit Checklist

### Checklist Requirements

Pre-Audit Checklist:

A reminder to provide us with the must haves 3 business days prior to the audit start days to avoid delays:

**Must haves:**

- A URL of the repository containing the source code
- The release branch / commit hash to be reviewed
- An explicit list of files in scope and out of scope for the security audit
- Robust and comprehensive documentation describing the intended functionality of the system

**Nice-to-haves:**

- Clear instructions for setting up the system and run the tests (usually in the README file)
- Any past audits
- Any tooling output logs
- Output generated by running the test suite
- Test coverage report

Please disregard what may be irrelevant in the above list for this particular audit.
Reminder that we cannot accept any scope changes during the course of the audit.

And more on audit preparation in this blogpost: https://medium.com/thesis-defense/why-crypto-needs-security-audits-d12f3909ac21 - thanks!

### Checklist Response

**Repository**: https://github.com/pokt-network/smt

- **Branch**: `main`
- **Hash**: `868237978c0b3c0e2added161b36eeb7a3dc93b0`

**Documentation**

- **Background**: [Relay Mining](https://arxiv.org/abs/2305.10672)
- **Technical Documentation**: https://github.com/pokt-network/smt/tree/main/docs
- _NOTE: we may integrate this into [https://dev.poktroll.com](https://dev.poktroll.com/) (which is out of scope) in the future_

**Files**

- Nothing is explicitly out of scope but the focus should be on the files below
- The following is a manually filtered list of files after running `tree -P '*.go' -I '*_test.go'`

```bash
.
├── errors.go
├── hasher.go
├── kvstore
│   ├── badger
│   │   ├── errors.go
│   │   ├── godoc.go
│   │   ├── interface.go
│   │   └── kvstore.go
│   ├── interfaces.go
├── options.go
├── proofs.go
├── smst.go
├── smt.go
├── types.go
└── utils.go
```

**Makefile**

- Running `make` in the root of the repo shows a list of options
- This gives access to tests, benchmarks, etc...

```bash
make
help Prints all the targets in all the Makefiles
list List all make targets
test_all runs the test suite
test_badger runs the badger KVStore submodule's test suite
mod_tidy runs go mod tidy for all (sub)modules
go_docs Generate documentation for the project
benchmark_all runs all benchmarks
benchmark_smt runs all benchmarks for the SMT
benchmark_smt_fill runs a benchmark on filling the SMT with different amounts of values
benchmark_smt_ops runs the benchmarks testing different operations on the SMT against different sized tries
benchmark_smst runs all benchmarks for the SMST
benchmark_smst_fill runs a benchmark on filling the SMST with different amounts of values
benchmark_smst_ops runs the benchmarks test different operations on the SMST against different sized tries
benchmark_proof_sizes runs the benchmarks test the proof sizes for different sized tries
```
27 changes: 27 additions & 0 deletions docs/faq.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# FAQ <!-- omit in toc -->

- [History](#history)
- [Fork](#fork)
- [Implementation](#implementation)
- [What's the story behind Extension Node Implementation?](#whats-the-story-behind-extension-node-implementation)

This documentation is meant to capture common questions that come up and act
as a supplement or secondary reference to the primary documentation.

## History

### Fork

This library was originally forked off of [celestiaorg/smt](https://github.com/celestiaorg/smt)
which was archived on Feb 27th, 2023.

## Implementation

### What's the story behind Extension Node Implementation?

The [SMT extension node](./smt.md#extension-nodes) is very similar to that of
Ethereum's [Modified Merkle Patricia Trie](https://ethereum.org/developers/docs/data-structures-and-encoding/patricia-merkle-trie).

A quick primer on it can be found in this [5P;1R post](https://olshansky.substack.com/p/5p1r-ethereums-modified-merkle-patricia).

WIP
28 changes: 16 additions & 12 deletions docs/mapstore.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# MapStore

<!-- toc -->
# MapStore <!-- omit in toc -->

- [Introduction](#introduction)
- [Implementations](#implementations)
* [SimpleMap](#simplemap)
* [BadgerV4](#badgerv4)
- [SimpleMap](#simplemap)
- [BadgerV4](#badgerv4)
- [Note On External Writability](#note-on-external-writability)

<!-- tocstop -->
## Introduction

The `MapStore` is a simple interface used by the SM(S)T to store, delete and
retrieve key-value pairs. It is intentionally simple and minimalistic so as to
Expand All @@ -31,11 +31,15 @@ See [simplemap.go](../kvstore/simplemap/simplemap.go) for more details.

### BadgerV4

This library provides a wrapper around [dgraph-io/badger][badgerv4] to adhere
to the `MapStore` interface. See the [full documentation](./badger-store.md)
for additional functionality and implementation details.
This library provides a wrapper around [dgraph-io/badger][https://github.com/dgraph-io/badger] to adhere to
the `MapStore` interface. See the [full documentation](./badger-store.md) for
additional functionality and implementation details.

See: [badger](../kvstore/badger/) for more details on the implementation of this
submodule.

See: [badger](../kvstore/badger/) for more details on the implementation of
this submodule.
## Note On External Writability

[badgerv4]: https://github.com/dgraph-io/badger
Any key-value store used by the tries should **not** be able to be externally
writeable in production. This opens the possibility to attacks where the writer
can modify the trie database and prove values that were not inserted.
Loading
Loading