Skip to content

Conversation

@facundominguez
Copy link
Collaborator

@facundominguez facundominguez commented Oct 31, 2025

This PR modifies updates updateCertDeposits so pool deposits are not counted again when reregistering a pool. This is the function used by the UTXO rule to update deposits.

The change to the function is fairly simple, but then there are a few broken proofs that are fixed in later commits.

Comment on lines 139 to 142
open import Relation.Binary.Reasoning.PartialOrder ⊆-Poset
open import Relation.Binary.Structures using (IsEquivalence)
module ≡ᵉA = IsEquivalence (≡ᵉ-isEquivalence {A = A})
open import Relation.Binary.Reasoning.Syntax
open ⊆-syntax _IsRelatedTo_ _IsRelatedTo_ ≤-go public
Copy link
Collaborator Author

@facundominguez facundominguez Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setup is longer than the proof. Maybe we should create a module
Axiom.Set.Reasoning to reexport these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option is just to use the same style as in the proofs below:

  dom∪ˡˡ a∈ = begin→ a∈ $
    a ∈ dom (m ˢ)                   →⟨ ∪-⊆ˡ ⟩
    a ∈ dom (m ˢ) ∪ dom (rhs-∪ˡ ˢ)  →⟨ proj₂ dom∪ ⟩
    a ∈ dom ((m ˢ) ∪ (rhs-∪ˡ ˢ))    →⟨ id ⟩
    a ∈ dom ((m ∪ˡ m') ˢ)
    ∎→

Copy link
Collaborator Author

@facundominguez facundominguez Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the latest version I rewrote this with Function.Reasoning._|>_:. Thanks to @javierdiaz72 for pointing it.

@WhatisRT suggested using EquivalenceReasoning from Function.Related.Propositional, but I couldn't figure out how to use it with implication yet.

  dom∪ˡˡ {a} a∈ = a∈ |> id ∶
      a ∈ dom (m ˢ)                   |> ∪-⊆ˡ ∶
      a ∈ dom (m ˢ) ∪ dom (rhs-∪ˡ ˢ)  |> proj₂ dom∪ ∶
      a ∈ dom ((m ˢ) ∪ (rhs-∪ˡ ˢ))    |> id ∶
      a ∈ dom ((m ∪ˡ m') ˢ)

Comment on lines 145 to 153
_∎→ : ∀ (A : Type) → A → A
A ∎→ = id
infix 3 _∎→

_→⟨_⟩_ : ∀ (x : Type) {y z : Type} → (x → y) → (y → z) → x → z
_ →⟨ f ⟩ g = g ∘ f
infixr 2 _→⟨_⟩_

begin→ : ∀ {A B : Type} → A → (A → B) → B
begin→ a f = f a
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are used in the proofs below. Do they belong here or in some more general place?

@facundominguez facundominguez force-pushed the fd/reregistration-deposits branch 3 times, most recently from 8f29a25 to 7a868c3 Compare November 3, 2025 15:47
@facundominguez facundominguez force-pushed the fd/reregistration-deposits branch 4 times, most recently from 90c7453 to 5f87d42 Compare November 8, 2025 18:12
Copy link
Collaborator

@carlostome carlostome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I'd suggest to move the auxiliary lemmata that belongs to agda-sets to src-lib-exts/agda-sets to (1) not clutter main files, and (2) contain those definitions that do not really belong to ledger in a specific place.

Copy link
Member

@williamdemeo williamdemeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good. I just pushed one small fix to mkdocs site (correct nav toc to reflect new location of Test module).

@williamdemeo
Copy link
Member

I should have just created a separate issue and small PR to fix the nav toc issue instead of pushing the fix to your PR. (Sorry!) I'm creating the "mkdocs nav fix" PR now and hoping that if we merge that into master first, then rebase your PR on master, there won't be any serious conflicts and the nav issue will be resolved.

@facundominguez facundominguez force-pushed the fd/reregistration-deposits branch from 5688a2b to 40e4a82 Compare November 11, 2025 21:24
@facundominguez
Copy link
Collaborator Author

I rebased master and updated agda-sets. Maybe this is ready to merge now.

Copy link
Collaborator

@carlostome carlostome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to go modulo removing those lemmas that correspond to the PR input-output-hk/agda-sets#19

@facundominguez
Copy link
Collaborator Author

I think it is better to merge this now and continue the work on moving proofs to agda-sets in other PRs. Or at least, I don't see the benefit of putting everything together in a single PR.

It is looking like more contributions need to be done in agda-sets. For instance, putting the moved proofs in opaque blocks to save some typechecking time.

@facundominguez facundominguez merged commit d802bb5 into master Nov 12, 2025
14 checks passed
@facundominguez facundominguez deleted the fd/reregistration-deposits branch November 12, 2025 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants