Move vote delegation removal to GovCert - #1288
Conversation
aa33569 to
471a052
Compare
williamdemeo
left a comment
There was a problem hiding this comment.
Here's something Claude found that I'm not sure about, but I'll post it in case it helps you.
Generated Haskell no longer compiles.
CertState moved modules
src/Ledger/Conway/Foreign/Certs.agda:83-84src/Ledger/Dijkstra/Foreign/Certs.agda:62-63
Moving HsTy-CertState/Conv-CertState out of Foreign/Cert.agda moves the generated Haskell datatype with it.
In this PR's hs artifact, data CertState = MkCertState… appears only in MAlonzo/Code/Ledger/{Conway,Dijkstra}/Foreign/Certs.hs and is gone from Foreign/Cert.hs. But the static re-export modules still import it from the old location:
build-tools/static/hs-src/src/MAlonzo/Code/Ledger/Conway/Foreign/API.hs:14-15build-tools/static/hs-src/src/MAlonzo/Code/Ledger/Dijkstra/Foreign/API.hs:19-20
MAlonzo emits modules without export lists, so the Agda-level public on the import does not re-export the Haskell type and GHC will reject import …Foreign.Cert (…, CertState(..)).
CI will not flag it: the hs target only assembles the tree, and build-tools/nix/hs-src.nix checks nothing beyond the presence of the .cabal file. Both API.hs files need CertState(..) imported from Foreign.Certs.
|
Another Claude find seems useful: The GOVCERT step is now resolved by an ambiguous instance search
Computational-GOVCERT and Computational-CERT now have the same index type Please make it explicit: I suspect this is also why the |
| `DRep`{.AgdaInductiveConstructor} is reset. | ||
|
|
||
| + `GOVCERT-deregdrep`{.AgdaInductiveConstructor} deregisters a | ||
| `DRep`{.AgdaInductiveConstructor}. |
There was a problem hiding this comment.
| `DRep`{.AgdaInductiveConstructor}, and every vote delegation to that `DRep`{.AgdaInductiveConstructor} is dropped. |
There was a problem hiding this comment.
This was meant to suggest replacing the period with a comma and adding the phrase that begins, "and every vote..."
471a052 to
8d97e41
Compare
| GOVCERTFromConf .convⁱ _ (C.GOVCERT-regdrep h) = C.GOVCERT-regdrep h | ||
| GOVCERTFromConf .convⁱ _ (C.GOVCERT-deregdrep (h , _)) = C.GOVCERT-deregdrep h | ||
| GOVCERTFromConf .convⁱ _ (C.GOVCERT-ccreghot h) = C.GOVCERT-ccreghot h |
There was a problem hiding this comment.
This is a pre-existing flaw, not something this PR introduced, but we might as well fix it here, since it sits in the middle of what the PR touches.
In GOVCERTFromConf the right-hand sides are C.GOVCERT-… where I think L.GOVCERT-… is meant (cf. DELEGFromConf above).
8d97e41 to
08a6af8
Compare
|
Confirmed the two fixes: Remaining items, none of them blocking: Docs. Small things.
Maybe worth recording. Pruning at deregistration makes |
williamdemeo
left a comment
There was a problem hiding this comment.
Nice work, Carlos! Approving, but see the (non-blocking) comments I added in case you want to address any of them here before merging.
08a6af8 to
fb6a5f6
Compare
Description
This PR changes the Conway (and Dijkstra) specification to match the implementation.
In particular, in the implementation a DRep deregistering certificate makes the DRep delegated stake be reset to empty, even if the same DRep is reregistered in the same transaction.
Checklist
CHANGELOG.md