Feat/stacks signer #3850

jcnelson · 2023-08-18T20:09:36Z

This PR adds in the event observer system for StackerDB chunk arrivals, and adds a libsigner crate which consumes them and feeds them into a signer-specific runloop. StackerDB events contain the chunk data, so downstream consumers do not need to go and pull them.

libsigner is a basic crate that just implements an event observer client for StackerDB messages, and a means of passing them into an implementation-specific runloop body. See the unit test in libsigner/src/tests/mod.rs for example usage.

codecov · 2023-08-18T20:10:48Z

Codecov Report

Merging #3850 (904c7ab) into develop (5ca6fd2) will decrease coverage by 0.01%.
Report is 1 commits behind head on develop.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #3850      +/-   ##
===========================================
- Coverage     0.16%    0.16%   -0.01%     
===========================================
  Files          322      339      +17     
  Lines       287661   290230    +2569     
===========================================
  Hits           469      469              
- Misses      287192   289761    +2569

Files Changed	Coverage Δ
libsigner/src/error.rs	`0.00% <0.00%> (ø)`
libsigner/src/events.rs	`0.00% <0.00%> (ø)`
libsigner/src/http.rs	`0.00% <0.00%> (ø)`
libsigner/src/runloop.rs	`0.00% <0.00%> (ø)`
libsigner/src/session.rs	`0.00% <0.00%> (ø)`
libsigner/src/tests/http.rs	`0.00% <0.00%> (ø)`
libsigner/src/tests/mod.rs	`0.00% <0.00%> (ø)`
libstackerdb/src/libstackerdb.rs	`0.00% <ø> (ø)`
stacks-common/src/deps_common/ctrlc/error.rs	`0.00% <ø> (ø)`
stacks-common/src/deps_common/ctrlc/mod.rs	`0.00% <ø> (ø)`
... and 23 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

…projects (like the signer binaries)

…NS not being available

…f DNS

…db for now)

kantai

The stacks signer seems like it could use a request library like ureq or reqwest rather than implementing a new HTTP client.

jcnelson · 2023-08-21T21:20:10Z

The stacks signer seems like it could use a request library like ureq or reqwest rather than implementing a new HTTP client.

Sure; I just added a really simple one for testing / debugging purposes. The stacker-signer will code to the trait, not this particular implementation.

…signers will need them too

…would use this crate to implement a Stacks event receiver and a runloop.

Signed-off-by: Jacinta Ferrant <[email protected]>

… in CI

Implement full DkgSign over StackerDB

…s interested; remove sleep calls in test since we know when it's done

Signed-off-by: Jacinta Ferrant <[email protected]>

jcnelson · 2023-09-20T20:08:55Z

@kantai @stjepangolemac @xoloki @jferrant There are a few PRs that are blocked on this PR. Can we merge this to develop? It's becoming a staging ground for a bunch of signer PRs that really should be hitting develop. Thanks!

xoloki

Looks good! My only real comment is that it would be great to remove as much custom HTTP code as possible, since we're using tiny_http internally now in libsigner. But I'd be happy if this all went in as-is.

I can also submit upstream PRs to tiny_http to remove any unsafe code there. I'd good at playing the open source game ;)

xoloki · 2023-09-20T20:12:43Z

libsigner/src/http.rs

+
+/// Decoding of the relevant parts of a signer-directed HTTP request from the Stacks node
+#[derive(Debug)]
+pub struct SignerHttpRequest {


Since we're using tiny_http now, probably all of this can go away.

Did that happen in an upstream PR? Or is this just now dead code?

Either way, would it make sense to first merge the PR that includes tiny_http into this PR first, and then merge this PR with tiny_http applied?

This happened in the first Signer PR:

https://github.com/stacks-network/stacks-blockchain/blob/feat/stacks-signer/libsigner/Cargo.toml#L29
https://github.com/stacks-network/stacks-blockchain/blob/feat/stacks-signer/libsigner/src/events.rs#L33
https://github.com/stacks-network/stacks-blockchain/blob/feat/stacks-signer/libsigner/src/events.rs#L201

We're currently using it as an HTTP server for StackerDBEventReceiver. So all of the server side HTTP parsing can go away in libsigner, since tiny_http comes with its own parser and request/response objects, along with all the tests.

In a later PR we can explore getting rid of async_h1, which is used for the client side of the stacks event observer code.

So AFAICT all of the libsigner custom HTTP code is now dead.

Lines 33 through 127 are dead. However, we do use the run_http_request code (rpc_request is used throughout SignerSession which in turn uses run_http_request and a few other of the functions here)

xoloki · 2023-09-20T20:14:13Z

libsigner/src/tests/http.rs

+
+use stacks_common::util::chunked_encoding::*;
+
+#[test]


This can also probably all go away since we're using tiny_http

Lines 29 through 101 can but until we update SignerSession, we can't get rid of the rest.

stjepangolemac

I haven't seen anything obviously wrong or weird, but the PR is large so take this with a grain of salt.

Signed-off-by: Jacinta Ferrant <[email protected]>

jferrant

Aside from potentially deleting the portions of dead HTTP code, might want to make a ticket to fully remove it from SignerSession and to use tiny http instead. Aside from that would just merge and build smaller PRs off of Develop.

So TLDR. LGTM

blockstack-devops · 2024-11-10T00:21:50Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

jcnelson requested review from kantai and jferrant August 18, 2023 20:09

jcnelson added 7 commits August 18, 2023 16:11

refactor: put httparse into stacks-common so it can be used in other …

0dbe22e

…projects (like the signer binaries)

Merge branch 'feat/stackerdb-rpc' into feat/stacks-signer

aa6bcd1

fix: fix comment

74389d8

chore: update imports

08e876a

fix: clear bootstrap node in testnet so we don't fail a test due to D…

555c19f

…NS not being available

fix: clear bootstrap nodes for tests so we don't fail due to a lack o…

de927bd

…f DNS

feat: prototype stacks-signer daemon (just a CLI front-end to stacker…

8b22d19

…db for now)

kantai reviewed Aug 18, 2023

View reviewed changes

jcnelson added 2 commits August 21, 2023 17:29

chore: fix failing test case

0c203ca

fix: remove commented-out code

469ab96

jcnelson changed the base branch from feat/stackerdb-rpc to feat/stackerdb-event-observer August 23, 2023 01:27

jcnelson added 13 commits August 22, 2023 21:34

Merge branch 'feat/stackerdb-event-observer' into feat/stacks-signer

95ab139

fix: fix compile-time error in unit tests due to new API

944d3a5

chore: cargo fmt

2ade75e

Merge branch 'feat/stackerdb-event-observer' into feat/stacks-signer

f50a49a

Merge branch 'feat/stackerdb-event-observer' into feat/stacks-signer

4f53702

refactor: move all remaining vendored deps into stacks_common, since …

be79b1a

…signers will need them too

refactor: HTTP code will be moved to a separate libsigner crate

ea67dc0

feat: new libsigner implementation. Stacker signers and sBTC signers …

4e306ce

…would use this crate to implement a Stacks event receiver and a runloop.

chore: update cargo deps with new vendored deps' needs

21144da

chore: update Cargo.lock with refactored vendored deps

29543fc

chore: libsigner is now a compilation unit

199d5ed

refactor: use libsigner

a773687

chore: vendored ctrlc deps live in stacks_common

8a01733

jcnelson marked this pull request as ready for review August 28, 2023 15:46

fix: include demo target

26e12d7

jferrant and others added 18 commits September 15, 2023 15:31

Fix hanging of thread due to failure to send data to the listener

9c5416a

Signed-off-by: Jacinta Ferrant <[email protected]>

Clippy fixes for libsigner

8bee21e

Signed-off-by: Jacinta Ferrant <[email protected]>

Fix accidental typo in forward_event during clippy changes

6d7e05a

Signed-off-by: Jacinta Ferrant <[email protected]>

try setting shm size so we can keep building in tmpfs

3f36dbc

try without tmpfs

dff0001

add libclang-dev to Dockerfile.large-genesis to fix full genesis test…

7b5694e

… in CI

had to add rustfmt to large genesis dockerfile too

5318cbf

sort the chunk events in test_simple_signer

e52b3da

signer_id starts at 0 not 1

99c1218

Merge pull request #3911 from stacks-network/chore/port-dkg-signer-first

5dee84f

Implement full DkgSign over StackerDB

use a channel to send the results from the SignerRunLoop to whoever i…

d0999bf

…s interested; remove sleep calls in test since we know when it's done

Cleanup command and state machine within signer runloop

ff66188

Signed-off-by: Jacinta Ferrant <[email protected]>

Stop signers in test

505a31d

Signed-off-by: Jacinta Ferrant <[email protected]>

don't need to clone OperationResult, Signature, or SchnorrProof

18c6a60

CRC to retry commands on failure and clippy cleanup

82359be

Signed-off-by: Jacinta Ferrant <[email protected]>

CRC: assert the length of the sign and dkg results instead of warning

602564e

Signed-off-by: Jacinta Ferrant <[email protected]>

Clippy fixes for main.rs

0189324

Signed-off-by: Jacinta Ferrant <[email protected]>

Merge branch 'develop' into feat/stacks-signer

0bf6285

jcnelson requested review from kantai and xoloki September 20, 2023 20:08

xoloki approved these changes Sep 20, 2023

View reviewed changes

stjepangolemac approved these changes Sep 21, 2023

View reviewed changes

Cleanup clippy, add comments, and use thiserror

bbc91d2

Signed-off-by: Jacinta Ferrant <[email protected]>

jferrant approved these changes Sep 21, 2023

View reviewed changes

Merge branch 'develop' into feat/stacks-signer

904c7ab

jcnelson merged commit 7361ab4 into develop Sep 21, 2023
1 check passed

blockstack-devops added the locked label Nov 10, 2024

stacks-network locked as resolved and limited conversation to collaborators Nov 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/stacks signer #3850

Feat/stacks signer #3850

jcnelson commented Aug 18, 2023 •

edited

Loading

codecov bot commented Aug 18, 2023 •

edited

Loading

kantai left a comment

jcnelson commented Aug 21, 2023

jcnelson commented Sep 20, 2023

xoloki left a comment •

edited

Loading

xoloki Sep 20, 2023

jcnelson Sep 20, 2023

xoloki Sep 20, 2023

xoloki Sep 20, 2023

jferrant Sep 21, 2023 •

edited

Loading

xoloki Sep 20, 2023

jferrant Sep 21, 2023

stjepangolemac left a comment

jferrant left a comment

blockstack-devops commented Nov 10, 2024

Feat/stacks signer #3850

Feat/stacks signer #3850

Conversation

jcnelson commented Aug 18, 2023 • edited Loading

codecov bot commented Aug 18, 2023 • edited Loading

Codecov Report

kantai left a comment

Choose a reason for hiding this comment

jcnelson commented Aug 21, 2023

jcnelson commented Sep 20, 2023

xoloki left a comment • edited Loading

Choose a reason for hiding this comment

xoloki Sep 20, 2023

Choose a reason for hiding this comment

jcnelson Sep 20, 2023

Choose a reason for hiding this comment

xoloki Sep 20, 2023

Choose a reason for hiding this comment

xoloki Sep 20, 2023

Choose a reason for hiding this comment

jferrant Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

xoloki Sep 20, 2023

Choose a reason for hiding this comment

jferrant Sep 21, 2023

Choose a reason for hiding this comment

stjepangolemac left a comment

Choose a reason for hiding this comment

jferrant left a comment

Choose a reason for hiding this comment

blockstack-devops commented Nov 10, 2024

jcnelson commented Aug 18, 2023 •

edited

Loading

codecov bot commented Aug 18, 2023 •

edited

Loading

xoloki left a comment •

edited

Loading

jferrant Sep 21, 2023 •

edited

Loading