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

Implementation of deposit, requestExit and withdrawFunds functions #791

Merged

Conversation

philippecamacho
Copy link
Contributor

@philippecamacho philippecamacho commented Dec 1, 2023

Part of #737.

To test:

> just sol-test

Implements the three functions deposit, requestExit and withdrawFunds. Note that the queue logic for registration and exit will be addressed in another PR.

@philippecamacho philippecamacho marked this pull request as draft December 1, 2023 15:45
@philippecamacho philippecamacho changed the title Implementation of deposit, requestExitand ẁithdrawFunds` functions Implementation of deposit, requestExit and withdrawFunds functions Dec 1, 2023
@philippecamacho philippecamacho marked this pull request as ready for review December 1, 2023 23:53
Copy link
Contributor

@alxiong alxiong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think pending queue logic is little tricky when we support deposit.
for register and exit, we could set their registerEpoch and exitEpoch to the nextRegister/ExitEpoch() based on the queuing logic.
However, it's currently underspecified how this queue works, or who would clear the queue and when.
and when there's deposit, the current struct Node is not sufficient to capture "this node currently is active and has 10 token, but he has another 5 pending token to be active in epoch X down the line" info.
^^ we need to discuss this.

note: I haven't review the tests yet, since there are a few things to settle on contract logic first.

contracts/src/StakeTable.sol Outdated Show resolved Hide resolved
contracts/src/StakeTable.sol Show resolved Hide resolved
contracts/src/StakeTable.sol Outdated Show resolved Hide resolved
contracts/src/StakeTable.sol Outdated Show resolved Hide resolved
contracts/src/StakeTable.sol Outdated Show resolved Hide resolved
contracts/src/StakeTable.sol Outdated Show resolved Hide resolved
contracts/src/StakeTable.sol Outdated Show resolved Hide resolved
contracts/src/StakeTable.sol Outdated Show resolved Hide resolved
contracts/src/StakeTable.sol Outdated Show resolved Hide resolved
@philippecamacho
Copy link
Contributor Author

philippecamacho commented Dec 5, 2023

However, it's currently underspecified how this queue works, or who would clear the queue and when

The way the queue is specified does not require any cleanup process: the queue just tracks two numbers that are updated when next{Register/Exit}Epoch is called.

struct Node is not sufficient to capture "this node currently is active and has 10 token, but he has another 5 pending token to be active in epoch X down the line

Yes, I agree. The current specification allows for balances to change within an epoch and that feels wrong. Let us discuss.

cc @jbearer

@philippecamacho
Copy link
Contributor Author

I think pending queue logic is little tricky when we support deposit.

@alxiong The queue logic (for both Registration and Exit) is implemented in #803.

@philippecamacho
Copy link
Contributor Author

However, it's currently underspecified how this queue works, or who would clear the queue and when

The way the queue is specified does not require any cleanup process: the queue just tracks two numbers that are updated when next{Register/Exit}Epoch is called.

struct Node is not sufficient to capture "this node currently is active and has 10 token, but he has another 5 pending token to be active in epoch X down the line

Yes, I agree. The current specification allows for balances to change within an epoch and that feels wrong. Let us discuss.

cc @jbearer

Based on the zulip conversation, I would add a test to ensure a user can deposit several times inside an epoch when the node is active.

@philippecamacho philippecamacho merged commit feb4b40 into main Dec 7, 2023
9 checks passed
@philippecamacho philippecamacho deleted the feat/t737-stable-table-deposit-requestExit-withdrawFunds branch December 7, 2023 16:59
alxiong added a commit that referenced this pull request Dec 22, 2023
* Set up a different module sub-command for each type of persistence

* Create a trait to abstract persistence-specific functionality

The new trait encapsulates
* Data source creation from command line options
* Looking up timestamp windows

It is currently implemented for FileSystemDataSource, but having
this trait will make it easy to incoporate the new SqlDataSource:
just implement the same trait.

* Break api into multiple single-purpose modules

api.rs has been very hard to work with, because almost all of the
logic, from initialization to endpoint handling, was in a single
massive, deeply nested function. Now there are smaller, less nested
modules separately handling initialization, endpoints, and state
updates. This should make it much easier to read, modify, and
extend the code.

* chore: use mocks/ instead of stubs/ (#790)

* Update query service

* Add support for SQL API storage

* Make API tests generic over storage type

* Instsantiate generic API tests for SQL storage

* Add an example of using the SQL query service to the local demo

Closes #584

* Document data source trait

* Appease cargo audit

* feat: add tx table entry proofs to tx payload proof (#796)

* add from_bytes, rename build -> from_txs

* add test infra for bad blocks

* add tx table proofs

* check tx table proofs in basic_correctness test

* tidy

* support zero-length txs

* flag new TODOs with TODO(795)

* tweak comments

* address https://github.com/EspressoSystems/espresso-sequencer/pull/796/files#r1414786947

* cache tx table len proof (whew)

* add TxInclusionProof::verify() as per #796 (comment)

* TxInclusionProof::tx_table_range_proof should always be a SINGLE RangeProof

* as per #796 (comment) add tx_table_len, tx_table_range_[start|end] to TxInclusionProof, remove 4 args from TxInclusionProof::verify()

* tidy

* Update dependencies

* Implementation of `deposit`, `requestExit` and `withdrawFunds` functions (#791)

---------

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>

* First pass at header/payload refactor

Nothing is building yet but the basic structure of the header and
payload should be evident

* Fix errors in HotShot initialization

* Fix errors in block impl

* Fix errors in API

* Complete first round of errors

* Fix more build errors

* Fix Clippy, except hotshot-testing deprecation warnings

* Fix tests

* Clean up hotshot-testing configuration

* Increase view timeout for hotshot test

* Make genesis header deterministic

* Decrease required decides for hotshot test

* Fix unused import warning

* Bump cachix/install-nix-action from 23 to 24

Bumps [cachix/install-nix-action](https://github.com/cachix/install-nix-action) from 23 to 24.
- [Release notes](https://github.com/cachix/install-nix-action/releases)
- [Commits](cachix/install-nix-action@v23...v24)

---
updated-dependencies:
- dependency-name: cachix/install-nix-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* Fix URL parsing in orchestrator and web servers

* Fix scripts to build docker images

- We now use a custom CARGO_TARGET_DIR in dev shells.
- The `cli` binary is no longer in this repo.

* Run natively with process-compose

Today during debugging I felt it was a bit painful that there wasn't a
quick and dirty way to run the full setup locally. This PR adds a
process-compose file that can be used to run the demo natively (except
for go-ethereum, which we usually don't debug).

I think this may come in handy the next time we want to debug, or
profile the application. The terminal UI looks pretty cool too.

* Fix local demo

- Cargo update
- Fix: https://github.com/EspressoSystems/HotShot/releases/tag/0.5.2

* Avoid using u64::MAX in timestamp tests

The Postgres backend only supports signed integer types up to 64
bits, so the largest timestamp it can represent is i64::MAX. This
should be fine, 63 bits is a whole lot of seconds.

* Enable status API without full query API

Closes #792

* Update sequencer/src/api.rs

Co-authored-by: Mathis <sveitser@gmail.com>

* Bump github/combine-prs from 4.1.0 to 5.0.0

Bumps [github/combine-prs](https://github.com/github/combine-prs) from 4.1.0 to 5.0.0.
- [Release notes](https://github.com/github/combine-prs/releases)
- [Commits](github/combine-prs@v4.1.0...v5.0.0)

---
updated-dependencies:
- dependency-name: github/combine-prs
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump cachix/cachix-action from 12 to 13 (#779)

Bumps [cachix/cachix-action](https://github.com/cachix/cachix-action) from 12 to 13.
- [Release notes](https://github.com/cachix/cachix-action/releases)
- [Commits](cachix/cachix-action@v12...v13)

---
updated-dependencies:
- dependency-name: cachix/cachix-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jeb Bearer <jeb.bearer@gmail.com>

* Update query service

* Bump itertools from 0.10.5 to 0.12.0

Bumps [itertools](https://github.com/rust-itertools/itertools) from 0.10.5 to 0.12.0.
- [Changelog](https://github.com/rust-itertools/itertools/blob/master/CHANGELOG.md)
- [Commits](rust-itertools/itertools@v0.10.5...v0.12.0)

---
updated-dependencies:
- dependency-name: itertools
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update HotShot and query service

* Remove workaround for decreasing timestamps

Now that timestamps are monotonically increasing, we can select all
blocks within a timestamp window in a much more direct and efficient
way.

Closes #794

* Fix handling of config path

* Load config from orchestrator or config path, if available
* Store config on startup
* Set default path in Dockerfile

* Remove config file saving

HotShot does this already

* Update surf-disco

This should allow the commit task to use WSS when connected to the
sequencer via HTTPS, which _may_ fix problems we've been having
with HTTPS.

* Update surf-disco

This should enable TLS for WebSockets clients

* Appease cargo audit

* Update sequencer/api/migrations/V11__timestamp_index.sql

Co-authored-by: Mathis <sveitser@gmail.com>

* Comments explaining test helpers

* Fix from()

* Update HotShot

* flake.lock: Update (#742)

Flake lock file updates:

• Updated input 'fenix':
    'github:nix-community/fenix/ec493cf412f94155daac4b95c95eb11ddcb347e5' (2023-11-04)
  → 'github:nix-community/fenix/aa632e1b140686853a226fa0bf85ae8ebbf72aab' (2023-12-09)
• Updated input 'fenix/rust-analyzer-src':
    'github:rust-lang/rust-analyzer/0fec61aabf62faab0c9f9b33b40ea5d5977792c8' (2023-11-03)
  → 'github:rust-lang/rust-analyzer/19387d3077c4c81e4a89ecec62917221fed26541' (2023-12-08)
• Updated input 'flake-utils':
    'github:numtide/flake-utils/ff7b65b44d01cf9ba6a71320833626af21126384' (2023-09-12)
  → 'github:numtide/flake-utils/4022d587cbbfd70fe950c1e2083a02621806a725' (2023-12-04)
• Updated input 'foundry':
    'github:shazow/foundry.nix/fc064153ac002e825724ff2091cd91e7d501ffef' (2023-11-04)
  → 'github:shazow/foundry.nix/ad6182c16c85a3303cb97ecd37086b034510a302' (2023-12-04)
• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/fa804edfb7869c9fb230e174182a8a1a7e512c40' (2023-11-02)
  → 'github:NixOS/nixpkgs/2c7f3c0fb7c08a0814627611d9d7d45ab6d75335' (2023-12-04)
• Updated input 'nixpkgs-cross-overlay':
    'github:alekseysidorov/nixpkgs-cross-overlay/df4bac108819562090817e96b94cf7dfef44e69c' (2023-11-04)
  → 'github:alekseysidorov/nixpkgs-cross-overlay/8744b817e5fbe80f9f77f819572edb4f0366f20a' (2023-11-28)
• Updated input 'nixpkgs-cross-overlay/nixpkgs':
    'github:NixOS/nixpkgs/fa804edfb7869c9fb230e174182a8a1a7e512c40' (2023-11-02)
  → 'github:NixOS/nixpkgs/5a09cb4b393d58f9ed0d9ca1555016a8543c2ac8' (2023-11-24)
• Updated input 'nixpkgs-cross-overlay/rust-overlay':
    'github:oxalica/rust-overlay/321affd863e3e4e669990a1db5fdabef98387b95' (2023-11-03)
  → 'github:oxalica/rust-overlay/055d3d2ea161dfc6ca569f2f135a107f48cf483e' (2023-11-27)
• Updated input 'nixpkgs-cross-overlay/treefmt-nix':
    'github:numtide/treefmt-nix/5deb8dc125a9f83b65ca86cf0c8167c46593e0b1' (2023-10-27)
  → 'github:numtide/treefmt-nix/e82f32aa7f06bbbd56d7b12186d555223dc399d1' (2023-11-12)
• Updated input 'pre-commit-hooks':
    'github:cachix/pre-commit-hooks.nix/dec10399e5b56aa95fcd530e0338be72ad6462a0' (2023-11-01)
  → 'github:cachix/pre-commit-hooks.nix/e5ee5c5f3844550c01d2131096c7271cec5e9b78' (2023-11-25)
• Updated input 'rust-overlay':
    'github:oxalica/rust-overlay/321affd863e3e4e669990a1db5fdabef98387b95' (2023-11-03)
  → 'github:oxalica/rust-overlay/2cfb76b8e836a26efecd9f853bea78355a11c58a' (2023-12-09)
• Updated input 'solc-bin':
    'github:EspressoSystems/nix-solc-bin/00868cfebdda3e3471386d3e5f35d1a55e57145f' (2023-05-22)
  → 'github:EspressoSystems/nix-solc-bin/bd5fcb1d247e0b1bac1fe5a3defa9df3e86b8f1e' (2023-11-29)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Update contract bindings

Most likely required after solc update in #742

* Run formatters

* Update HotShot

* feat: Support tx proofs for truncated tx payloads and negative-length txs (#846)

* new test infra for making block payloads

* new test malformed_payloads, fix bug in tx_payload_range

* malformed_payloads test cases are now payload bytes

* wip add failing test, debug logs

* fix: check tx range against payload byte len

* test_vid_factory return impl Trait

* TxInclusionProof::verify check consistency of vid_common against vid_commit

* check tx_payload_range empty when payload proof is absent

* accommodate changes to jellyfish API

* more test cases, refactor test code

* tidy tests

* remove jellyfish patch in Cargo.toml

* remove debug logs from production code

* add more test cases

* partially fix CI

* fix test infra, address PR comments

* Fix native demo

* Fix just command
* Switch some nodes to file system query service
* Run status API only on other nodes

* Refactor/t775 update dependency bn254 (#797)

* Use forge for testing the BLS signature implementation in solidity.
* Remove BN256.G2 dependency and related code.
* Remove function *verifyAggSig*.
* Point to solidity-bn254 v0.2.0.
* Use G2ParsedPoint in diff_test.rs.

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>

* Bump zerocopy from 0.7.29 to 0.7.31

Bumps [zerocopy](https://github.com/google/zerocopy) from 0.7.29 to 0.7.31.
- [Release notes](https://github.com/google/zerocopy/releases)
- [Changelog](https://github.com/google/zerocopy/blob/main/CHANGELOG.md)
- [Commits](google/zerocopy@v0.7.29...v0.7.31)

---
updated-dependencies:
- dependency-name: zerocopy
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Update HotShot, query service, and Tide Disco

* Add batches for required CI jobs

* Feat/t737 queue logic (#803)

* Mark functions `nextRegistrationEpoch` / `nextExitEpoch` as view 
* Add `appendRegistrationQueue` and `appendExitQueue` functions  to update the queues.
* Fuzzing test function `testFuzz_SequencesOfEvents` that simulate scenarios with registrations, exit requests and epochs updates.

---------

Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>

* Fix Header::new and add tests

I noticed a bug where we were not preventing the l1_head or
l1_finalized block from decreasing, due to L1 clients between
nodes being out of sync.

Added a bunch of new tests for the monotonicity properties of
various header fields. Note that the test `test_header_invariants`
(more of an integration test) is not very stringent yet, because
all the nodes are using the same (fake) L1 client. However, once
HotShot's stateful header stuff is complete, it should be possible
to give each node its own L1 client, even within the same process,
and then this test will become much more useful. In the meantime,
we now have good unit test coverage.

* Add tool to check consistency of headers in a chain

* parameterize the WS polling interval

* fmt

* command line duration

* fmt

* Update HotShot

* Update HotShot

* Update status tests

* feat: Support block payloads that are too small to hold the entire tx table (#870)

* add test for correctness of block.iter()

* add test for expected number of txs in a block

* refactor test

* tidy test

* add failing test case, more test tidying

* test pass but watch out for overflow

* add test with huge tx table len

* accommodate extremely small payloads

* test infra allow larger payload, not just truncated payload

* begin refactoring test code

* tidy test

* tidy tests

* add TODO(817) comments

* add test for malicious tx inclusion proof

* Recreate HotShot deployment script and deploy contract

* flake.lock: Update (#871)

Flake lock file updates:

• Updated input 'fenix':
    'github:nix-community/fenix/aa632e1b140686853a226fa0bf85ae8ebbf72aab' (2023-12-09)
  → 'github:nix-community/fenix/8001e62f21df863304287c63f01fcc0c24c91816' (2023-12-16)
• Updated input 'fenix/rust-analyzer-src':
    'github:rust-lang/rust-analyzer/19387d3077c4c81e4a89ecec62917221fed26541' (2023-12-08)
  → 'github:rust-lang/rust-analyzer/21b06c1beb9bb59369ffd652f5d617bcf6952e05' (2023-12-15)
• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/2c7f3c0fb7c08a0814627611d9d7d45ab6d75335' (2023-12-04)
  → 'github:NixOS/nixpkgs/a9bf124c46ef298113270b1f84a164865987a91c' (2023-12-11)
• Updated input 'pre-commit-hooks':
    'github:cachix/pre-commit-hooks.nix/e5ee5c5f3844550c01d2131096c7271cec5e9b78' (2023-11-25)
  → 'github:cachix/pre-commit-hooks.nix/007a45d064c1c32d04e1b8a0de5ef00984c419bc' (2023-12-13)
• Updated input 'rust-overlay':
    'github:oxalica/rust-overlay/2cfb76b8e836a26efecd9f853bea78355a11c58a' (2023-12-09)
  → 'github:oxalica/rust-overlay/2a186e207c9ef8e3eef114259044fc2f92043d59' (2023-12-16)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Improve logging/feedback

* Remove `connect(None)`, which waits indefinitely for a successful
  connection to the server. With this script, we should always be
  connecting to a server that has been up and running for some time.
  So failure to connect indicates a problem like the URL being wrong,
  not something likely to resolve itself. Now we just panic in this
  case.
* Add INFO level log telling how many headers we plan to check

This fixes a problem where there was no visible feedback whether the
script was working or hanging. Now, at INFO level you will see the
initial message, and you will see something at WARN level if anything
goes wrong. This keeps the logging pretty quiet while still being
able to tell if things are working.

* Make header test cases more reasonable

* Update query service and improve SQL backend

* Use new Query API for database queries
* Make database resettable
* Fix some tests, which were failing based on timing

Closes #843

* Bump time from 0.3.30 to 0.3.31 (#881)

Bumps [time](https://github.com/time-rs/time) from 0.3.30 to 0.3.31.
- [Release notes](https://github.com/time-rs/time/releases)
- [Changelog](https://github.com/time-rs/time/blob/main/CHANGELOG.md)
- [Commits](time-rs/time@v0.3.30...v0.3.31)

---
updated-dependencies:
- dependency-name: time
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* backoff on number of leaves when gas limit is exceeded

* Bump anyhow from 1.0.75 to 1.0.76

Bumps [anyhow](https://github.com/dtolnay/anyhow) from 1.0.75 to 1.0.76.
- [Release notes](https://github.com/dtolnay/anyhow/releases)
- [Commits](dtolnay/anyhow@1.0.75...1.0.76)

---
updated-dependencies:
- dependency-name: anyhow
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Bump async-trait from 0.1.74 to 0.1.75

Bumps [async-trait](https://github.com/dtolnay/async-trait) from 0.1.74 to 0.1.75.
- [Release notes](https://github.com/dtolnay/async-trait/releases)
- [Commits](dtolnay/async-trait@0.1.74...0.1.75)

---
updated-dependencies:
- dependency-name: async-trait
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Fix tests that do not run locally by waiting for anvil endpoints to be up. (#883)

Ensure the anvil endpoints are up in `AnvilOptions::spawn`.

* use sync error to test for gas limit, incorporate a soft block limit that we can increase if we succeed

Signed-off-by: nomaxg <noahgolub2@gmail.com>

* populate error with num leaves sent

Signed-off-by: nomaxg <noahgolub2@gmail.com>

* Add a tool to reset persistent storage

Closes #887

* Update Tide Disco and HotShot

* Run *cargo update*

* Try without rust cache

* feat: VID namespace `from_txs` support namespaces (#891)

* add test for correctness of block.iter()

* add test for expected number of txs in a block

* refactor test

* tidy test

* add failing test case, more test tidying

* test pass but watch out for overflow

* add test with huge tx table len

* accommodate extremely small payloads

* test infra allow larger payload, not just truncated payload

* begin refactoring test code

* tidy test

* tidy tests

* add TODO(817) comments

* add test for malicious tx inclusion proof

* NamespaceTable stub

* WIP: add namespace table to payload, need to update tests

* fix test

* WIP rework basic_correctness test for multiple namespaces

* WIP fixing bugs

* WIP begin tidying tests

* tidy tests

* tidy

* be less stupid as per #891 (comment)

* Revert "update hotshot dep to use latest main"

This reverts commit 3cf2ab2.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: nomaxg <noahgolub2@gmail.com>
Co-authored-by: Jeb Bearer <jeb@espressosys.com>
Co-authored-by: Alex Xiong <alex.xiong.tech@gmail.com>
Co-authored-by: Jeb Bearer <jeb.bearer@gmail.com>
Co-authored-by: Gus Gutoski <ggutoski@users.noreply.github.com>
Co-authored-by: Philippe Camacho <philippe@espressosys.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Rob <rob@espressosys.com>
Co-authored-by: rob-maron <132852777+rob-maron@users.noreply.github.com>
Co-authored-by: nomaxg <noahgolub2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants