Skip to content

Commit aae99d0

Browse files
authored
Merge pull request #6429 from wileyj/chore/release_signer_3.2.0.0.1.1_intermediate
Merge release/signer-3.2.0.0.1.1 to develop (replaces 6413)
2 parents bc9adb3 + 48c48e5 commit aae99d0

File tree

20 files changed

+196
-40
lines changed

20 files changed

+196
-40
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
99

1010
### Added
1111

12+
- Add `stackerdb_timeout_secs` to miner config for limiting duration of StackerDB HTTP requests.
1213
- When determining a global transaction replay set, the state evaluator now uses a longest-common-prefix algorithm to find a replay set in the case where a single replay set has less than 70% of signer weight.
1314

1415
### Changed

libsigner/src/session.rs

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
use std::net::TcpStream;
1818
use std::str;
19+
use std::time::Duration;
1920

2021
use clarity::vm::types::QualifiedContractIdentifier;
2122
use libstackerdb::{
@@ -103,22 +104,34 @@ pub struct StackerDBSession {
103104
pub stackerdb_contract_id: QualifiedContractIdentifier,
104105
/// connection to the replica
105106
sock: Option<TcpStream>,
107+
/// The timeout applied to HTTP read and write operations
108+
socket_timeout: Duration,
106109
}
107110

108111
impl StackerDBSession {
109112
/// instantiate but don't connect
110-
pub fn new(host: &str, stackerdb_contract_id: QualifiedContractIdentifier) -> StackerDBSession {
113+
pub fn new(
114+
host: &str,
115+
stackerdb_contract_id: QualifiedContractIdentifier,
116+
socket_timeout: Duration,
117+
) -> StackerDBSession {
111118
StackerDBSession {
112119
host: host.to_owned(),
113120
stackerdb_contract_id,
114121
sock: None,
122+
socket_timeout,
115123
}
116124
}
117125

118126
/// connect or reconnect to the node
119127
fn connect_or_reconnect(&mut self) -> Result<(), RPCError> {
120128
debug!("connect to {}", &self.host);
121-
self.sock = Some(TcpStream::connect(&self.host)?);
129+
let sock = TcpStream::connect(&self.host)?;
130+
// Make sure we don't hang forever if for some reason our node does not
131+
// respond as expected such as failing to properly close the connection
132+
sock.set_read_timeout(Some(self.socket_timeout))?;
133+
sock.set_write_timeout(Some(self.socket_timeout))?;
134+
self.sock = Some(sock);
122135
Ok(())
123136
}
124137

@@ -251,11 +264,49 @@ impl SignerSession for StackerDBSession {
251264
/// upload a chunk
252265
fn put_chunk(&mut self, chunk: &StackerDBChunkData) -> Result<StackerDBChunkAckData, RPCError> {
253266
let body =
254-
serde_json::to_vec(chunk).map_err(|e| RPCError::Deserialize(format!("{:?}", &e)))?;
267+
serde_json::to_vec(chunk).map_err(|e| RPCError::Deserialize(format!("{e:?}")))?;
255268
let path = stackerdb_post_chunk_path(self.stackerdb_contract_id.clone());
256269
let resp_bytes = self.rpc_request("POST", &path, Some("application/json"), &body)?;
257270
let ack: StackerDBChunkAckData = serde_json::from_slice(&resp_bytes)
258-
.map_err(|e| RPCError::Deserialize(format!("{:?}", &e)))?;
271+
.map_err(|e| RPCError::Deserialize(format!("{e:?}")))?;
259272
Ok(ack)
260273
}
261274
}
275+
276+
#[cfg(test)]
277+
mod tests {
278+
use std::io::Write;
279+
use std::net::TcpListener;
280+
use std::thread;
281+
282+
use super::*;
283+
284+
#[test]
285+
fn socket_timeout_works_as_expected() {
286+
let listener = TcpListener::bind("127.0.0.1:0").expect("bind failed");
287+
let addr = listener.local_addr().unwrap();
288+
289+
let short_timeout = Duration::from_millis(200);
290+
thread::spawn(move || {
291+
if let Ok((mut stream, _)) = listener.accept() {
292+
// Sleep long enough so the client should hit its timeout
293+
std::thread::sleep(short_timeout * 2);
294+
let _ = stream.write_all(b"HTTP/1.1 200 OK\r\n\r\n");
295+
}
296+
});
297+
298+
let contract_id = QualifiedContractIdentifier::transient();
299+
let mut session = StackerDBSession::new(&addr.to_string(), contract_id, short_timeout);
300+
301+
session.connect_or_reconnect().expect("connect failed");
302+
303+
// This should fail due to the timeout
304+
let result = session.rpc_request("GET", "/", None, &[]);
305+
match result {
306+
Err(RPCError::IO(e)) => {
307+
assert_eq!(e.kind(), std::io::ErrorKind::WouldBlock);
308+
}
309+
other => panic!("expected timeout error, got {other:?}"),
310+
}
311+
}
312+
}

stacks-node/src/nakamoto_node/miner.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,11 @@ impl BlockMinerThread {
11061106
NakamotoNodeError::MinerConfigurationFailed("Failed to get RPC loopback socket")
11071107
})?;
11081108
let miners_contract_id = boot_code_id(MINERS_NAME, chain_state.mainnet);
1109-
let mut miners_session = StackerDBSession::new(&rpc_socket.to_string(), miners_contract_id);
1109+
let mut miners_session = StackerDBSession::new(
1110+
&rpc_socket.to_string(),
1111+
miners_contract_id,
1112+
self.config.miner.stackerdb_timeout,
1113+
);
11101114

11111115
if Self::fault_injection_skip_block_push() {
11121116
warn!(

stacks-node/src/nakamoto_node/signer_coordinator.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,11 @@ impl SignerCoordinator {
107107
.get_rpc_loopback()
108108
.ok_or_else(|| ChainstateError::MinerAborted)?;
109109
let miners_contract_id = boot_code_id(MINERS_NAME, is_mainnet);
110-
let miners_session = StackerDBSession::new(&rpc_socket.to_string(), miners_contract_id);
110+
let miners_session = StackerDBSession::new(
111+
&rpc_socket.to_string(),
112+
miners_contract_id,
113+
config.miner.stackerdb_timeout,
114+
);
111115

112116
// build a BTreeMap of the various timeout steps
113117
let mut block_rejection_timeout_steps = BTreeMap::<u32, Duration>::new();

stacks-node/src/nakamoto_node/stackerdb_listener.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,11 @@ impl StackerDBListener {
179179
.node
180180
.get_rpc_loopback()
181181
.ok_or_else(|| ChainstateError::MinerAborted)?;
182-
let mut signers_session =
183-
StackerDBSession::new(&rpc_socket.to_string(), signers_contract_id.clone());
182+
let mut signers_session = StackerDBSession::new(
183+
&rpc_socket.to_string(),
184+
signers_contract_id.clone(),
185+
config.miner.stackerdb_timeout,
186+
);
184187
let entries: Vec<_> = signer_entries.values().cloned().collect();
185188
let parsed_entries = SignerEntries::parse(config.is_mainnet(), &entries)
186189
.expect("FATAL: could not parse retrieved signer entries");

stacks-node/src/neon_node.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2302,8 +2302,11 @@ impl BlockMinerThread {
23022302
/// Only used in mock signing to determine if the peer info view was already signed across
23032303
fn mock_block_exists(&self, peer_info: &PeerInfo) -> bool {
23042304
let miner_contract_id = boot_code_id(MINERS_NAME, self.config.is_mainnet());
2305-
let mut miners_stackerdb =
2306-
StackerDBSession::new(&self.config.node.rpc_bind, miner_contract_id);
2305+
let mut miners_stackerdb = StackerDBSession::new(
2306+
&self.config.node.rpc_bind,
2307+
miner_contract_id,
2308+
self.config.miner.stackerdb_timeout,
2309+
);
23072310
let miner_slot_ids: Vec<_> = (0..MINER_SLOT_COUNT * 2).collect();
23082311
if let Ok(messages) = miners_stackerdb.get_latest_chunks(&miner_slot_ids) {
23092312
for message in messages.into_iter().flatten() {
@@ -2379,8 +2382,11 @@ impl BlockMinerThread {
23792382
let stackerdbs = StackerDBs::connect(&self.config.get_stacker_db_file_path(), false)
23802383
.map_err(|e| e.to_string())?;
23812384
let miner_contract_id = boot_code_id(MINERS_NAME, self.config.is_mainnet());
2382-
let mut miners_stackerdb =
2383-
StackerDBSession::new(&self.config.node.rpc_bind, miner_contract_id);
2385+
let mut miners_stackerdb = StackerDBSession::new(
2386+
&self.config.node.rpc_bind,
2387+
miner_contract_id,
2388+
self.config.miner.stackerdb_timeout,
2389+
);
23842390
let miner_db = MinerDB::open_with_config(&self.config).map_err(|e| e.to_string())?;
23852391

23862392
SignerCoordinator::send_miners_message(

stacks-node/src/tests/nakamoto_integrations.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,11 @@ pub fn get_latest_block_proposal(
487487
let miner_ranges = stackerdb_conf.signer_ranges();
488488
let latest_miner = usize::from(miner_info.get_latest_winner_index());
489489
let miner_contract_id = boot_code_id(MINERS_NAME, false);
490-
let mut miners_stackerdb = StackerDBSession::new(&conf.node.rpc_bind, miner_contract_id);
490+
let mut miners_stackerdb = StackerDBSession::new(
491+
&conf.node.rpc_bind,
492+
miner_contract_id,
493+
Duration::from_secs(30),
494+
);
491495

492496
let mut proposed_blocks: Vec<_> = stackerdb_conf
493497
.signers
@@ -12781,29 +12785,28 @@ fn write_signer_update(
1278112785
) {
1278212786
let signers_contract_id =
1278312787
MessageSlotID::StateMachineUpdate.stacker_db_contract(false, reward_cycle);
12784-
let mut session = StackerDBSession::new(&conf.node.rpc_bind, signers_contract_id);
12788+
let mut session = StackerDBSession::new(
12789+
&conf.node.rpc_bind,
12790+
signers_contract_id,
12791+
Duration::from_secs(30),
12792+
);
1278512793
let message = SignerMessageV0::StateMachineUpdate(update);
1278612794

12787-
// Submit the block proposal to the signers slot
12788-
let mut accepted = false;
12795+
// Submit the update to the signers slot
1278912796
let mut version = 0;
12790-
let start = Instant::now();
12791-
while !accepted {
12797+
wait_for(timeout.as_secs(), || {
1279212798
let mut chunk =
1279312799
StackerDBChunkData::new(signer_slot_id, version, message.serialize_to_vec());
1279412800
chunk
1279512801
.sign(&signer_sk)
1279612802
.expect("Failed to sign message chunk");
1279712803
debug!("Produced a signature: {:?}", chunk.sig);
1279812804
let result = session.put_chunk(&chunk).expect("Failed to put chunk");
12799-
accepted = result.accepted;
1280012805
version += 1;
1280112806
debug!("Test Put Chunk ACK: {result:?}");
12802-
assert!(
12803-
start.elapsed() < timeout,
12804-
"Timed out waiting for signer state update to be accepted"
12805-
);
12806-
}
12807+
Ok(result.accepted)
12808+
})
12809+
.expect("Failed to accept signer state update");
1280712810
}
1280812811

1280912812
/// Test SIP-031 activation

stacks-node/src/tests/signer/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,6 +1485,7 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
14851485
self.get_current_reward_cycle(),
14861486
SignerSlotID(0), // We are just reading so again, don't care about index.
14871487
SignerDb::new(":memory:").unwrap(),
1488+
Duration::from_secs(30),
14881489
);
14891490
let mut latest_msgs = StackerDB::get_messages(
14901491
stackerdb
@@ -1549,6 +1550,7 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
15491550
reward_cycle,
15501551
SignerSlotID(0), // We are just reading so again, don't care about index.
15511552
SignerDb::new(":memory:").unwrap(), // also don't care about the signer db for version tracking
1553+
Duration::from_secs(30),
15521554
)
15531555
}
15541556

@@ -1593,6 +1595,7 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
15931595
.expect("Failed to get signer slot id")
15941596
.expect("Signer does not have a slot id"),
15951597
SignerDb::new(":memory:").unwrap(),
1598+
Duration::from_secs(30),
15961599
);
15971600

15981601
let signature = private_key

stacks-node/src/tests/signer/v0.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -492,8 +492,11 @@ impl SignerTest<SpawnedSigner> {
492492
/// Propose a block to the signers
493493
fn propose_block(&self, block: NakamotoBlock, timeout: Duration) {
494494
let miners_contract_id = boot_code_id(MINERS_NAME, false);
495-
let mut session =
496-
StackerDBSession::new(&self.running_nodes.conf.node.rpc_bind, miners_contract_id);
495+
let mut session = StackerDBSession::new(
496+
&self.running_nodes.conf.node.rpc_bind,
497+
miners_contract_id,
498+
self.running_nodes.conf.miner.stackerdb_timeout,
499+
);
497500
let burn_height = self
498501
.running_nodes
499502
.btc_regtest_controller
@@ -17853,8 +17856,11 @@ fn miner_stackerdb_version_rollover() {
1785317856
max_chunk.slot_version
1785417857
);
1785517858

17856-
let mut stackerdb =
17857-
StackerDBSession::new(&conf_2.node.rpc_bind, boot_code_id(MINERS_NAME, false));
17859+
let mut stackerdb = StackerDBSession::new(
17860+
&conf_2.node.rpc_bind,
17861+
boot_code_id(MINERS_NAME, false),
17862+
conf_2.miner.stackerdb_timeout,
17863+
);
1785817864

1785917865
let proposals_before = miners.get_primary_proposals_submitted().get();
1786017866

stacks-signer/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to the versioning scheme outlined in the [README.md](README.md).
77

8+
89
## Unreleased
910

1011
### Added
1112

1213
- When determining a global transaction replay set, the state evaluator now uses a longest-common-prefix algorithm to find a replay set in the case where a single replay set has less than 70% of signer weight.
1314

15+
## [3.2.0.0.1.1]
16+
17+
### Added
18+
19+
- Introduced `stackerdb_timeout_secs`: config option to set the maximum time (in seconds) the signer will wait for StackerDB HTTP requests to complete.
20+
1421
## [3.2.0.0.1.0]
1522

1623
### Changed

0 commit comments

Comments
 (0)