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

chore: mock RotateKeysTransactions with consistent addresses #833

Merged
merged 3 commits into from
Nov 14, 2024
Merged

chore: mock RotateKeysTransactions with consistent addresses #833

merged 3 commits into from
Nov 14, 2024

Conversation

Jiloc
Copy link
Collaborator

@Jiloc Jiloc commented Nov 13, 2024

Description

Change the mocking of the RotateKeysTransaction address field in the tests added in #801. The tests were valid but used the bitcoin aggregate_key to compute the address. The changes aim to improve clarity and correctness and avoid confusion for future contributors.

Changes

  • Replaced the incorrect mocking of StacksAddress with the correct value from the wallet when available, or used StacksAddress::from_public_keys when it was not.

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

@Jiloc Jiloc added the chore label Nov 13, 2024
@Jiloc Jiloc requested review from djordon and matteojug November 13, 2024 12:54
djordon
djordon approved these changes Nov 13, 2024
View reviewed changes
Copy link
Contributor

@djordon djordon left a comment

Choose a reason for hiding this comment

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

Looks good, left one suggestion about setting the aggregate key.

signer/src/testing/dummy.rs Outdated Show resolved Hide resolved
@Jiloc Jiloc merged commit ec2002f into main Nov 14, 2024
5 checks passed
AshtonStephens pushed a commit that referenced this pull request Nov 18, 2024
@Jiloc @AshtonStephens
chore: mock RotateKeysTransactions with consistent addresses (#833)
d9694a0 
* mock RotateKeysTransactions with consistent addresses

* randomize test aggregate_key
aldur added a commit that referenced this pull request Nov 19, 2024
@AshtonStephens @matteojug
@djordon @Jiloc @kyranjamie @MCJOHN974 @cylewitruk @xoloki @aldur
feat: Add production like docker compose for docs (#813)
dda01eb 
* feat: Add production like docker compose for docs

* chore: Add disclaimers and more configuration variables

* feat: Add bitcoin node with special params

* nit: Fix nits.

* fix: add missing parameter to complete deposit call (#795)

* feat: Retry updates when a version conflict fails update (#753)

* feat: Retry updates when a version conflict fails update

* chore: fix linter errors

* feat: Put async Emily updates back

* fix: Do some Emily updates sequentially

* chore: fix nits

* fix: Fix unit tests I broke

* Submit rotate-key tx (#788)

* feat: submit rotate-key tx

* feat: add functions for validating deposit prevouts part 2 (#749)

* Add a function to the DbRead trait for fetching the deport request report.

* add parsing and handling of rotate-keys events (#799)

* add parsing and handling of rotate-keys events

* address review comments

* chore: commit generated sources (#806)

* chore: use nextest (#807)

* Use nextest in CI.
* Move one libp2p test to the integration test folder

* feat: add protobuf conversion impls (#808)

* move stuff out of the mod.rs file
* Add conversion impls for crypto module protobufs and some of the id types
* Finish the bitcoin types
* Finish the conversion for common stacks types
* Add tests for conversions
* Finish off signer decision types

* feat: implement validation of deposit fees (#803)

* Add a new fee assessment trait
* Add the max fee variable to the deposit request report
* Add fee validation function and unit tests

* feat: add functions to help create sighashes (#805)

* add a new table for signer outputs
* Simplify the implementation of the get_signer_utxo function
* Make sure donations are written to the new table
* make sure the bitcoin_blockchain_of function returns the right number of rows.
* Add function for getting the btc state
* Add full function for validating a deposit

* Add rotate keys tx validation (#815)

* feat: add rotate keys tx validation

* feat: deconstruct and store bitcoin transactions into inputs and outputs (#822)

* Add new tables for bitcoin inputs and outputs
* Add trait for deconstructing transactions into the input and output types
* Add functions for writing transaction inputs and outputs to the database
* Use the new tables for get_utxo
* Remove the SignerOutput type and signer_txos table
* Update the block observer to write to the new tables

* feat: 801 feature extend db schemas to save all data from new block print events (#828)

* add support for new data in sbtc print events

* close #726

* chore: demo-cli (#825)

* fix store_rotate_keys test (#830)

* Address codegen lint (#831)

* fix: add codegen fmt after build
* chore: remove codegen dependency on `lint`
* chore: fmt

* chore: dead link in readme (#832)

* Add alternative blocklist API (#812)

[feature] Add alternative API to blocklist client

* chore: add codegen fmt after (#835)

* 814 feature add api key security to private endpoints on emily openapi spec (#816)

* feat: API Key security for private Emily Api calls

* chore: Lint and format

* feat: delete persistent data on dev stacks deletion (#819)

* feat: API Key security for private Emily Api calls

* chore: Lint and format

* feat: delete persistent data on dev stacks deletion

* chore: change autogenerated code

* 823 feature integrate emily api with aws api gateway (#824)

* feat: Deploy lambda binary correctly

* feat: handle stage name prefixing in api routes

* chore: feature guard testing functions and imports

* feat: Add stack deploy task to emily cdk project

* chore: Remove unused import

* chore: cargo fmt

* fix: Fix expected lambda architecture test

* feat: get last bitcoin fees from sweep tables (#809) (#810)

* get fees from sweep tables

* update sweep query

* updates to query etc

* store tx vsize on unsigned tx

* pop.unwrap for readability

* update comment

* fixes and tests

* comments

* comment and method rename

* forgot to drop test db

* update comments

* pr nit

* handle forks

* didnt need a separate fn

* fix get fees

* move integration tests

* might as well use try_fold here too

* return none if size is 0

* keep divide by zero error

* update tests after get_signer_utxo changes from main

* couple of tests showing errors in get_fees()

* generated stuff accidentally snuck in

* [chore] Introduce cargo vet (#827)

[chore] Introduce cargo vet

* check that wsts messages are signed by the specified signer_id (#723)

* add coordinator checks

* once we have a chain tip, use the correct signer as coordinator

* use HashMap::get rather than index directly; replace resulting copypasta with macro

* move message authentication into separate fn

* pass chain tip report into handle_wsts_message

* clippy fixes

* use non-panic hashmap access; convert macro to closure

* no need to ref when calling hashmap::get

* fix weird cargo fmt spacing on tracing call; warn about wrong coordinator then return Ok

* resolve scoping issue with warn

* feat: impl bitcoin-core's `gettxspendingprevout` and `getmempooldescendants` RPCs (#759 part 1) (#834)

* wip

* wip

* wip

* additional test and docs

* add impls to bitcoincoreclient

* revert error enum

* rename outpoint+comment+separate code

* outpoint already imported

* pr nits

* little refactor of rpc method

* missed bitcoin block gen 100

* chore: mock RotateKeysTransactions with consistent addresses (#833)

* mock RotateKeysTransactions with consistent addresses

* randomize test aggregate_key

* feat: run blocklist client for deposits that we cannot sign for (#838)

* Change the name of the is_accepted column in the deposit_signers table
* Update the queries
* Update the types

* feat: add withdrawal request report types and stubs (#841)

* Sort withdrawal requests by their qualified request id
* Add withdrawal request types for validation
* Add stubs for the withdrawal request queries

* feat: Add blocklist client publishing to dockerhub (#844)

* fix: incompatible target name and use debug release (#845)

* feat: Add API key generation to CDK (#821)

* feat: API Key security for private Emily Api calls

* chore: Lint and format

* feat: delete persistent data on dev stacks deletion

* feat: Add API key generation to CDK

* nit: Explain usage plan throttle

* chore: Address nit.

* fix: Fix dumb merging

* chore: Address nit

* chore: add devenv demo steps (#851)

* fix: no more flaky `load_latest_deposit_requests_persists_requests_from_past` test (#850)

* Make sure we wait for an up-to-date database in the test.
* Make sure errors in the block observer don't hold up this test forever

* use dev images and add  to postres alpine version

* fix: undo nits

* chore: fix nits

* Specify Dockerfile

---------

Co-authored-by: Matteo Almanza <[email protected]>
Co-authored-by: Daniel Jordon <[email protected]>
Co-authored-by: Jiloc <[email protected]>
Co-authored-by: kyranjamie <[email protected]>
Co-authored-by: Viktar Makouski <[email protected]>
Co-authored-by: Cyle Witruk <[email protected]>
Co-authored-by: Joey Yandle <[email protected]>
Co-authored-by: djordon <[email protected]>
Co-authored-by: Adriano Di Luzio <[email protected]>
@Jiloc Jiloc deleted the chore/improve-rotate-keys-address-mocking branch November 25, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djordon djordon djordon approved these changes

@matteojug matteojug Awaiting requested review from matteojug

Assignees
No one assigned
Labels
chore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Jiloc @djordon