[Bug] CachedNonceManager should not be Clone #1507
Description
Component
provider, pubsub
What version of Alloy are you on?
alloy-provider v0.4.2 (main branch)
Operating System
Linux
Describe the bug
Problem description
CachedNonceManager keeps DashMap for local storing of the current nonce per account. DashMap implements Clone (https://docs.rs/dashmap/latest/src/dashmap/lib.rs.html#96-110) by simply coping every entry. Therefore, two clones of a CachedNonceManager instance will fall into a conflict after just one transaction.
The right way of using it
As in the unit test (https://github.com/alloy-rs/alloy/blob/main/crates/provider/src/fillers/nonce.rs#L213-L214), any provider using nonce caching underneath MUST be kept under Arc. However, lib API doesn't enforce that and thus it is very easy (and pretty natural) to clone raw providers and counting that the nonce caching will be shared across clones.
If these clones share signer (e.g. WalletFiller), then a nonce conflict is inevitable (unless provider is always cloned before sending any transaction, but then we don't benefit from local caching, since the nonce is always being fetched).
General solution
IMHO the problem lies in the totally weird impl Clone for DashMap and thus I suggest two ways of keeping users safe:
- change
DashMapto anything reasonable, where cloning actually preserves entries - make
CachedNonceManager!Clone
Reproduction of a very easy misuse
#[tokio::test]
async fn cloned_caching_does_not_cooperate() {
let filler = NonceFiller::<CachedNonceManager>::default();
let filler_copy = filler.clone();
let provider = ProviderBuilder::new().on_anvil();
let address = Address::ZERO;
assert_eq!(
filler.nonce_manager.get_next_nonce(&provider, address).await.unwrap(),
0
);
assert_eq!(
filler_copy.nonce_manager.get_next_nonce(&provider, address).await.unwrap(),
1
);
}