Integrate KES agent functionality into ouroboros-consensus #1620
Conversation
fraser-iohk
requested review from
nfrisby,
jasagredo,
amesgen,
dnadales and
geo2a
as code owners
fraser-iohk
force-pushed
the
fraser-iohk/kes-agent-rebase-v0.1.0.0
branch
4 times, most recently
from
012585e to
b38cdd3
Compare
fraser-iohk
force-pushed
the
fraser-iohk/kes-agent-rebase-v0.1.0.0
branch
from
d5634f7 to
8023c8d
Compare
...-consensus-cardano/src/unstable-cardano-testlib/Test/ThreadNet/Infra/ShelleyBasedHardFork.hs
Outdated
Show resolved
Hide resolved
| instance | ||
| ( CardanoHardForkConstraints StandardCrypto | ||
| , IOLike m | ||
| , MonadKESAgent m |
There was a problem hiding this comment.
Should MonadKESAgent be part of IOLike?
There was a problem hiding this comment.
it could in theory, but MonadKESAgent is currently (in theory) Praos-specific and defined in ouroboros-consensus-diffusion (which is admittedly a bit weird) -- I can try to move it into ouroboros-consensus if that would make more sense
| @@ -158,7 +159,7 @@ synthesize genTxs DBSynthesizerConfig{confOptions, confShelleyGenesis, confDbDir | |||
| flavargs | |||
| $ ChainDB.defaultArgs | |||
|
|
|||
| forgers <- blockForging | |||
| (_, forgers) <- allocate registry (const $ mkForgers nullTracer) (mapM_ BlockForging.finalize) | |||
There was a problem hiding this comment.
We rely on the registry finalizing these keys. Are we manually finalizing them somewhere? If so, we should call release on the ResourceKey.
...os-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/Node/Tracers.hs
Show resolved
Hide resolved
| ( knownSlotWatcher btime $ | ||
| \currentSlot -> withRegistry (\rr -> withEarlyExit_ $ go rr currentSlot) | ||
| ) | ||
| (finalize blockForging) |
There was a problem hiding this comment.
Did we store this blockForging in some registry? Should we instead release it from there?
There was a problem hiding this comment.
yes, especially since HotKeys are not supposed to be finalized more than one time
...ensus-protocol/src/ouroboros-consensus-protocol/Ouroboros/Consensus/Protocol/Praos/Common.hs
Outdated
Show resolved
Hide resolved
| ( Just $ \handleKey handleDrop -> do | ||
| runKESAgentClient tr path handleKey handleDrop | ||
| ) | ||
| (pure ()) |
There was a problem hiding this comment.
Shouldn't the finalizer do something?
There was a problem hiding this comment.
afaik this should be fine -- we don't need to instruct the KES agent to forget the key since that occurs at the BlockForging level, and the HotKey's finalizer already cancels the async it spawns
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Block/Forging.hs
Outdated
Show resolved
Hide resolved
...nsensus/src/ouroboros-consensus/Ouroboros/Consensus/MiniProtocol/ChainSync/Client/Jumping.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Util/EarlyExit.hs
Outdated
Show resolved
Hide resolved
fraser-iohk
force-pushed
the
fraser-iohk/kes-agent-rebase-v0.1.0.0
branch
from
9022ee0 to
ccaa4c6
Compare
- Update to use newest cardano-crypto-class with unsound pure KES implementation - Use mlocked KES - Add KES agent connectivity - Rebase cleanup - Handle drop-key messages from KES Agent - Provide KESAgentClientTrace to BlockForging - Revert change to MockCrypto and require DSIGN only when running the KES agent - Bump kes-agent SRP to remove SerDoc dependency
…Source to make it unusable
fraser-iohk
force-pushed
the
fraser-iohk/kes-agent-rebase-v0.1.0.0
branch
from
ccaa4c6 to
7c41ce7
Compare
This PR supercedes #1487
includes the following squashed commit messages:
Update to use newest cardano-crypto-class with unsound pure KES implementation
Use mlocked KES
Add KES agent connectivity
Rebase cleanup
Handle drop-key messages from KES Agent
Provide KESAgentClientTrace to BlockForging
Revert change to MockCrypto and require DSIGN only when running the KES agent
Bump kes-agent SRP to remove SerDoc dependency
Description
Please include a meaningful description of the PR and link the relevant issues
this PR might resolve.
Also note that: