From a46890d3a4f3d67bc2d0c13b69972051748d58cf Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Fri, 12 Apr 2024 14:49:31 -0400 Subject: [PATCH] Reduce "shim types", introduce agreement.Block agreement.Block is similar to what was agreement.ProposableBlock, but rather than being an interface it is a concrete type. Therefore, there can be no confusion: A validated block is not an agreement.Block just because it has a Block() method. Instead, FinalizeBlock methods explicit copy their internal Block() objects into agreement.Block objects during finalization. This PR also eliminates node.validatedBlock as *ledgercore.ValidatedBlock implements agreement.ValidatedBlock already, no wrapper is needed. Blocks remain as immutable as before. --- agreement/abstractions.go | 17 +++++++---------- agreement/agreementtest/simulate_test.go | 4 ++-- agreement/common_test.go | 6 +++--- agreement/fuzzer/ledger_test.go | 4 ++-- agreement/player_permutation_test.go | 3 ++- agreement/proposal.go | 4 ++-- node/impls.go | 5 +++-- node/node.go | 20 ++------------------ 8 files changed, 23 insertions(+), 40 deletions(-) diff --git a/agreement/abstractions.go b/agreement/abstractions.go index e093e944fc..50ee6c10a3 100644 --- a/agreement/abstractions.go +++ b/agreement/abstractions.go @@ -84,23 +84,20 @@ type BlockFactory interface { // An UnfinishedBlock represents a Block produced by a BlockFactory // and must be finalized before being proposed by agreement. type UnfinishedBlock interface { - // WithSeed creates a copy of this UnfinishedBlock with its - // cryptographically random seed set to the given value. + // FinishBlock creates a Proposaable block, having set the cryptographically + // random seed and payout related fields. // // Calls to Seed() or to Digest() on the copy's Block must // reflect the value of the new seed. - FinishBlock(seed committee.Seed, proposer basics.Address, eligible bool) ProposableBlock + FinishBlock(seed committee.Seed, proposer basics.Address, eligible bool) Block Round() basics.Round } -// An ProposableBlock represents a Block produced by a BlockFactory, -// that was later finalized by providing the seed and the proposer, -// and can now be proposed by agreement. -type ProposableBlock interface { - // Block returns the underlying block that has been assembled. - Block() bookkeeping.Block -} +// A Block (in agreement) represents an UnfinishedBlock produced by a +// BlockFactory, that was later finalized by providing the seed and the +// proposer, and can now be proposed by agreement. +type Block bookkeeping.Block // A Ledger represents the sequence of Entries agreed upon by the protocol. // The Ledger consists of two parts: a LedgerReader and a LedgerWriter, which diff --git a/agreement/agreementtest/simulate_test.go b/agreement/agreementtest/simulate_test.go index f528ffca35..6c071ed8a8 100644 --- a/agreement/agreementtest/simulate_test.go +++ b/agreement/agreementtest/simulate_test.go @@ -83,13 +83,13 @@ func (b testValidatedBlock) Round() basics.Round { return b.Inside.Round() } -func (b testValidatedBlock) FinishBlock(s committee.Seed, proposer basics.Address, eligible bool) agreement.ProposableBlock { +func (b testValidatedBlock) FinishBlock(s committee.Seed, proposer basics.Address, eligible bool) agreement.Block { b.Inside.BlockHeader.Seed = s b.Inside.BlockHeader.Proposer = proposer if !eligible { b.Inside.BlockHeader.ProposerPayout = basics.MicroAlgos{} } - return b + return agreement.Block(b.Inside) } type testBlockValidator struct{} diff --git a/agreement/common_test.go b/agreement/common_test.go index f011bc26a6..ca8983705e 100644 --- a/agreement/common_test.go +++ b/agreement/common_test.go @@ -169,13 +169,13 @@ func (b testValidatedBlock) Round() basics.Round { return b.Inside.Round() } -func (b testValidatedBlock) FinishBlock(s committee.Seed, proposer basics.Address, eligible bool) ProposableBlock { +func (b testValidatedBlock) FinishBlock(s committee.Seed, proposer basics.Address, eligible bool) Block { b.Inside.BlockHeader.Seed = s b.Inside.BlockHeader.Proposer = proposer if !eligible { b.Inside.BlockHeader.ProposerPayout = basics.MicroAlgos{} } - return b + return Block(b.Inside) } type testBlockValidator struct{} @@ -538,7 +538,7 @@ func (v *voteMakerHelper) MakeRandomProposalPayload(t *testing.T, r round) (*pro pb := ub.FinishBlock(committee.Seed{}, basics.Address{}, false) var payload unauthenticatedProposal - payload.Block = pb.Block() + payload.Block = bookkeeping.Block(pb) payload.SeedProof = randomVRFProof() propVal := proposalValue{ diff --git a/agreement/fuzzer/ledger_test.go b/agreement/fuzzer/ledger_test.go index 6d5c91c28c..efd68ae087 100644 --- a/agreement/fuzzer/ledger_test.go +++ b/agreement/fuzzer/ledger_test.go @@ -97,13 +97,13 @@ func (b testValidatedBlock) Round() basics.Round { return b.Inside.Round() } -func (b testValidatedBlock) FinishBlock(s committee.Seed, proposer basics.Address, eligible bool) agreement.ProposableBlock { +func (b testValidatedBlock) FinishBlock(s committee.Seed, proposer basics.Address, eligible bool) agreement.Block { b.Inside.BlockHeader.Seed = s b.Inside.BlockHeader.Proposer = proposer if !eligible { b.Inside.BlockHeader.ProposerPayout = basics.MicroAlgos{} } - return b + return agreement.Block(b.Inside) } type testBlockValidator struct{} diff --git a/agreement/player_permutation_test.go b/agreement/player_permutation_test.go index 59d92e015f..216316d8bd 100644 --- a/agreement/player_permutation_test.go +++ b/agreement/player_permutation_test.go @@ -25,6 +25,7 @@ import ( "github.com/algorand/go-algorand/config" "github.com/algorand/go-algorand/crypto" "github.com/algorand/go-algorand/data/basics" + "github.com/algorand/go-algorand/data/bookkeeping" "github.com/algorand/go-algorand/data/committee" "github.com/algorand/go-algorand/protocol" "github.com/algorand/go-algorand/test/partitiontest" @@ -36,7 +37,7 @@ func makeRandomProposalPayload(r round) *proposal { pb := ub.FinishBlock(committee.Seed{}, basics.Address{}, false) var payload unauthenticatedProposal - payload.Block = pb.Block() + payload.Block = bookkeeping.Block(pb) payload.SeedProof = crypto.VRFProof{} return &proposal{unauthenticatedProposal: payload} diff --git a/agreement/proposal.go b/agreement/proposal.go index 73261dabd8..e696bfeb4b 100644 --- a/agreement/proposal.go +++ b/agreement/proposal.go @@ -102,8 +102,8 @@ type proposal struct { validatedAt time.Duration } -func makeProposalFromProposableBlock(blk ProposableBlock, pf crypto.VrfProof, origPer period, origProp basics.Address) proposal { - e := blk.Block() +func makeProposalFromProposableBlock(blk Block, pf crypto.VrfProof, origPer period, origProp basics.Address) proposal { + e := bookkeeping.Block(blk) var payload unauthenticatedProposal payload.Block = e payload.SeedProof = pf diff --git a/node/impls.go b/node/impls.go index b7ee54fab7..826f0399c4 100644 --- a/node/impls.go +++ b/node/impls.go @@ -26,6 +26,7 @@ import ( "github.com/algorand/go-algorand/data/basics" "github.com/algorand/go-algorand/data/bookkeeping" "github.com/algorand/go-algorand/ledger" + "github.com/algorand/go-algorand/ledger/ledgercore" "github.com/algorand/go-algorand/logging" "github.com/algorand/go-algorand/network" "github.com/algorand/go-algorand/util/execpool" @@ -59,7 +60,7 @@ func (i blockValidatorImpl) Validate(ctx context.Context, e bookkeeping.Block) ( return nil, err } - return validatedBlock{vb: lvb}, nil + return lvb, nil } // agreementLedger implements the agreement.Ledger interface. @@ -86,7 +87,7 @@ func (l agreementLedger) EnsureBlock(e bookkeeping.Block, c agreement.Certificat // EnsureValidatedBlock implements agreement.LedgerWriter.EnsureValidatedBlock. func (l agreementLedger) EnsureValidatedBlock(ve agreement.ValidatedBlock, c agreement.Certificate) { - l.Ledger.EnsureValidatedBlock(ve.(validatedBlock).vb, c) + l.Ledger.EnsureValidatedBlock(ve.(*ledgercore.ValidatedBlock), c) // let the network know that we've made some progress. l.n.OnNetworkAdvance() } diff --git a/node/node.go b/node/node.go index a608d4a5a1..88766877dc 100644 --- a/node/node.go +++ b/node/node.go @@ -1289,33 +1289,17 @@ func (node *AlgorandFullNode) SetCatchpointCatchupMode(catchpointCatchupMode boo } -// validatedBlock satisfies agreement.ValidatedBlock -type validatedBlock struct { - vb *ledgercore.ValidatedBlock -} - // unfinishedBlock satisfies agreement.UnfinishedBlock type unfinishedBlock struct { blk *ledgercore.UnfinishedBlock } -// proposableBlock satisfies agreement.ProposableBlock -type proposableBlock struct { - blk bookkeeping.Block -} - -// Block satisfies the agreement.ValidatedBlock interface. -func (vb validatedBlock) Block() bookkeeping.Block { return vb.vb.Block() } - // Round satisfies the agreement.UnfinishedBlock interface. func (ub unfinishedBlock) Round() basics.Round { return ub.blk.Round() } -// Block satisfies the agreement.ProposableBlock interface. -func (ab proposableBlock) Block() bookkeeping.Block { return ab.blk } - // FinishBlock satisfies the agreement.UnfinishedBlock interface. -func (ub unfinishedBlock) FinishBlock(s committee.Seed, proposer basics.Address, eligible bool) agreement.ProposableBlock { - return proposableBlock{blk: ub.blk.FinishBlock(s, proposer, eligible)} +func (ub unfinishedBlock) FinishBlock(s committee.Seed, proposer basics.Address, eligible bool) agreement.Block { + return agreement.Block(ub.blk.FinishBlock(s, proposer, eligible)) } // AssembleBlock implements Ledger.AssembleBlock.