STR-1331 Optimistic Fulfiller Duties Execution #137

Rajil1213 · 2025-05-08T19:35:32Z

Description

This PR adds the ability for operator nodes to fulfill withdrawal requests and to get reimbursed provided that they are not challenged.

In so doing, it also makes the following additional changes:

updates the strata dependency to use the updated checkpoint/chain-state structure.
updates the bitvm dependency to use the optimized graph generation.
updates most of the STFs to handle cases where a contract may receive txs belonging to other graphs tied to the same deposit.
updates the transaction APIs in the tx-graph crate to not require connectors when finalizing.
updates the dockerfile for bitcoind to automine blocks given a certain env variable.
handles an edge case where we proceed to the next step before all nonces are received for all graphs (previously, we only used to check the number of nonces per graph and not the number of graphs themselves).
updates the stake chain API to compute sighashes just-in-time with the supplied prevouts

TODO

rebase with main.
update the prover test files to fix failing tests.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Refactor
New or updated tests
Dependency Update

Notes to Reviewers

Good luck. 👀

Checklist

I have performed a self-review of my code.
I have commented my code where necessary.
I have updated the documentation if needed.
My changes do not introduce new warnings.
I have added (where necessary) tests that prove my changes are effective or that my feature works.
New and existing tests pass with my changes.

Related Issues

Closes STR-1331

Rajil1213 · 2025-05-08T21:49:16Z

I've rebased this with main and also updated the proof test-data. So this is actually ready for review now.

codecov · 2025-05-08T22:40:09Z

Codecov Report

Attention: Patch coverage is 21.64682% with 1513 lines in your changes missing coverage. Please review.

Project coverage is 51.87%. Comparing base (01a19e6) to head (e0e3c9f).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/duty-tracker/src/executors/deposit.rs	0.00%	399 Missing ⚠️
crates/duty-tracker/src/contract_state_machine.rs	0.00%	387 Missing ⚠️
...uty-tracker/src/executors/optimistic_withdrawal.rs	0.00%	330 Missing ⚠️
crates/duty-tracker/src/contract_manager.rs	0.00%	143 Missing ⚠️
crates/duty-tracker/src/contract_persister.rs	0.00%	75 Missing ⚠️
crates/agent/src/operator.rs	0.00%	42 Missing ⚠️
...ates/duty-tracker/src/stake_chain_state_machine.rs	0.00%	30 Missing ⚠️
crates/stake-chain/src/transactions/stake.rs	86.69%	29 Missing ⚠️
crates/bridge-proof/protocol/src/tx_info.rs	44.73%	21 Missing ⚠️
crates/stake-chain/src/stake_chain.rs	88.15%	9 Missing ⚠️
... and 13 more

@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
- Coverage   53.43%   51.87%   -1.57%     
==========================================
  Files         145      147       +2     
  Lines       24153    24650     +497     
==========================================
- Hits        12905    12786     -119     
- Misses      11248    11864     +616

Files with missing lines	Coverage Δ
bin/dev-cli/src/constants.rs	`0.00% <ø> (ø)`
crates/agent/src/base.rs	`0.00% <ø> (ø)`
crates/agent/src/strata_rpc.rs	`0.00% <ø> (ø)`
crates/bridge-proof/test-utils/src/lib.rs	`90.38% <100.00%> (ø)`
crates/connectors/src/connector_a3.rs	`93.27% <100.00%> (+0.10%)`	⬆️
crates/connectors/src/connector_k.rs	`100.00% <100.00%> (ø)`
crates/db/src/persistent/sqlite.rs	`94.84% <100.00%> (ø)`
crates/duty-tracker/src/predicates.rs	`64.40% <100.00%> (+0.45%)`	⬆️
crates/secret-service-proto/src/v1/traits.rs	`0.00% <ø> (ø)`
crates/secret-service-proto/src/v1/wire.rs	`45.45% <ø> (ø)`
... and 29 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ProofOfKeags

Beautifully organized PR. Content wise I have a lot to say, though. I'll leave the details to the individual comments. Any 🫡 emojis should be understood as explicit acks. This helps as we iterate on this PR to know what we've already looked at.

crates/duty-tracker/src/tx_driver.rs

crates/duty-tracker/src/contract_manager.rs

crates/duty-tracker/src/executors.rs

bin/secret-service/src/seeded_impl/wots.rs

crates/stake-chain/src/transactions/constants.rs

crates/duty-tracker/src/contract_state_machine.rs

crates/duty-tracker/src/contract_manager.rs

crates/db/src/persistent/sqlite.rs

crates/duty-tracker/src/executors/deposit.rs

storopoli

minor nits and agree with Keagan's review

crates/bridge-proof/protocol/src/tx_info.rs

crates/duty-tracker/src/contract_manager.rs

crates/operator-wallet/src/lib.rs

crates/secret-service-client/Cargo.toml

crates/secret-service-proto/Cargo.toml

storopoli · 2025-05-09T13:08:23Z

crates/stake-chain/src/transactions/stake.rs

+    /// # CAUTION
+    ///
+    /// This should only be invoked for stake_index >= 1, i.e. from at least second stake
+    /// transaction.


What if we added an idx field to StakeTx and then did an assert! here (or even a debug_assert!)?

We could but I don't know if that is any better -- we'd have panics either way. The only difference will be when we invoke this function rather than when we broadcast the transaction, and both those things happen at about the same time.

Cc @ProofOfKeags who might have a good argument here with enforcing invariants instead of warning about them in docstrings.

That's the thing -- adding a runtime assertion does not enforce the invariant any more than the docstring.

The only way to really enforce it is to have separate types for the first and subsequent stake txs.

Yes, agreed. Maybe add a TODO then or a ticket?

Okay, I'm cooking up something.

Okay check this out: f3ffd2a.

I've completely overhauled the stake tx API so that it distinguishes between the first one and the rest. It'll be easier to review in the following order:

crates/stake-chain/src/transactions/stake_tx.rs

crates/stake-chain/src/stake_chain.rs

duty-tracker/src/contract_state_machine.rs

duty-tracker/src/contract-manager.rs

duty-tracker/src/executors/optimistic_withdrawal.rs

This commit is amazing.

crates/tx-graph/src/transactions/post_assert.rs

crates/tx-graph/src/transactions/slash_stake.rs

storopoli

ACK 6f45030

crates/bridge-proof/protocol/src/tx_info.rs

crates/stake-chain/src/transactions/stake.rs

crates/duty-tracker/src/contract_state_machine.rs

…lment; refactor executors into separate module

…nment

crates/stake-chain/src/transactions/stake.rs

storopoli

ACK e0e3c9f

storopoli

ACK e0e3c9f

Rajil1213 requested review from ProofOfKeags and storopoli May 8, 2025 19:35

Rajil1213 self-assigned this May 8, 2025

Rajil1213 force-pushed the feature/fulfiller-duties branch from b313d9e to 29872d8 Compare May 8, 2025 21:47

Rajil1213 added P0 - Blocks Development required labels May 8, 2025

ProofOfKeags requested changes May 8, 2025

View reviewed changes

storopoli requested changes May 9, 2025

View reviewed changes

storopoli previously approved these changes May 9, 2025

View reviewed changes

ProofOfKeags reviewed May 9, 2025

View reviewed changes

crates/bridge-proof/protocol/src/tx_info.rs Show resolved Hide resolved

Rajil1213 dismissed storopoli’s stale review via 6d734d8 May 9, 2025 22:05

Rajil1213 force-pushed the feature/fulfiller-duties branch 3 times, most recently from 24cdf0a to 244ccdf Compare May 9, 2025 22:56

ProofOfKeags reviewed May 10, 2025

View reviewed changes

crates/stake-chain/src/transactions/stake.rs Outdated Show resolved Hide resolved

crates/stake-chain/src/transactions/stake.rs Outdated Show resolved Hide resolved

crates/duty-tracker/src/contract_state_machine.rs Outdated Show resolved Hide resolved

ProofOfKeags and others added 13 commits May 11, 2025 22:50

make connector k fields private, remove unused field

b74ae70

remove connector_k from ClaimTx::finalize arguments

7238c61

wip

61c6607

fix: remove unused vars/args

eba3963

feat/refactor(duty-tracker): add executor to handle withdrawal fulfil…

f7d1ccc

…lment; refactor executors into separate module

fix(s2): return bitvm::wots::Signature instead of flattened bytes

7e4e41c

fix(stake-chain): update docs on OPERATOR_FUNDS

3b1465b

refactor(duty-tracker): add a constants module for order of pks from s2

9015aed

feat(duty-tracker): handle duty to broadcast claim

580a2c4

refactor(tx-graph): make connectors a part of PayoutOptimisticTx

c373b55

feat(duty-tracker): handle broadcasting payout optimistic tx

a77a57f

refactor(tx-graph): use array of sigs for finalizing PayoutOptimisticTx

b165faa

chore(duty-tracker): fix typo

29d0014

Rajil1213 added 3 commits May 11, 2025 22:52

refactor(executors): clarify where VOUT comes from

239c684

refactor(duty-tracker): move constants to inside executors module

4dfe04c

fix: address review comments

e233451

Rajil1213 force-pushed the feature/fulfiller-duties branch 2 times, most recently from 2f05f4d to 8e1deca Compare May 11, 2025 17:26

Rajil1213 added 14 commits May 12, 2025 01:07

refactor(stake-chain): overhaul stake tx api

f3ffd2a

fix(docker): set BRIDGE_DENOMINATION to 10 btc

a0d58b9

chore(docker): reduce nag_interval and bury_depth

5bf961d

feat(duty-tracker): make multi-threaded graph gen opt-in

61ac14e

refactor(contract-manager): do not commit stake data upon every assig…

c148270

…nment

chore(persister): add log when persisting stake chain data

35a9228

docs(tx-driver): update doc after removing deadline param

2eb82df

chore(csm): plant flags where bodies are buried

27064e7

fix(dev-cli): update agg key to match params

35c8a0f

feat(csm): use macro to cache pog

d72399c

chore(persister): log more errors

4fd1f2e

chore: address reviews

906591b

refactor(stake-chain): add hash field in StakeTx to simplify advancement

c430131

fix: remove redundant refs after rebase

e0e3c9f

Rajil1213 force-pushed the feature/fulfiller-duties branch from 8e1deca to e0e3c9f Compare May 11, 2025 19:25

storopoli reviewed May 12, 2025

View reviewed changes

crates/stake-chain/src/transactions/stake.rs Show resolved Hide resolved

storopoli approved these changes May 12, 2025

View reviewed changes

Rajil1213 requested review from ProofOfKeags and storopoli May 12, 2025 18:26

storopoli approved these changes May 12, 2025

View reviewed changes

ProofOfKeags approved these changes May 12, 2025

View reviewed changes

Rajil1213 added this pull request to the merge queue May 12, 2025

Merged via the queue into main with commit ca7379d May 12, 2025
13 checks passed

Rajil1213 deleted the feature/fulfiller-duties branch May 12, 2025 19:27

STR-1331 Optimistic Fulfiller Duties Execution #137

STR-1331 Optimistic Fulfiller Duties Execution #137

Uh oh!

Conversation

Rajil1213 commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO

Type of Change

Notes to Reviewers

Checklist

Related Issues

Uh oh!

Rajil1213 commented May 8, 2025

Uh oh!

codecov bot commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ProofOfKeags left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

storopoli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

storopoli May 9, 2025

Choose a reason for hiding this comment

Uh oh!

Rajil1213 May 9, 2025

Choose a reason for hiding this comment

Uh oh!

storopoli May 9, 2025

Choose a reason for hiding this comment

Uh oh!

Rajil1213 May 9, 2025

Choose a reason for hiding this comment

Uh oh!

storopoli May 9, 2025

Choose a reason for hiding this comment

Uh oh!

Rajil1213 May 9, 2025

Choose a reason for hiding this comment

Uh oh!

Rajil1213 May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ProofOfKeags May 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

storopoli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

storopoli left a comment

Choose a reason for hiding this comment

Uh oh!

Rajil1213 commented May 8, 2025 •

edited

Loading

codecov bot commented May 8, 2025 •

edited

Loading

Rajil1213 May 9, 2025 •

edited

Loading