Skip to content

chore: update ootle docs - #198

Open
SWvheerden wants to merge 1 commit into
tari-project:mainfrom
SWvheerden:sw_audit_tari_ootle
Open

SWvheerden wants to merge 1 commit into
tari-project:mainfrom
SWvheerden:sw_audit_tari_ootle

Conversation

@SWvheerden

Copy link
Copy Markdown
Contributor

Description

updates the ootle docs

@SWvheerden SWvheerden changed the title choreL update ootle docs chore: update ootle docs Sep 15, 2026

@sdbondi sdbondi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Posted by Claude.

Review: chore: update ootle docs

I reviewed this against the tari-ootle implementation at f3c521e (current development HEAD). The restructuring is a clear improvement — moving these out of "Unknown status" into a proper The Tari Ootle section, dropping the Cerberus branding, and adding ootle.md all make the book much easier to navigate, and the bulk of the new technical detail checks out against the code (see the spot-check list at the end).

There are, however, five places where the new text describes mechanisms that don't exist in the implementation, three of which were true at some point and have since been removed or inverted. Since these RFCs are being promoted from "draft" to Status: Implemented / ![status: stable], they're worth fixing before merge.


Blocking

1. Node eviction was removed from the implementation

tari-ootle commit 62caafb — refactor!: remove node eviction (#2587) (10 Sep 2026, three days after the Last Modified date on these docs) removed the entire mechanism:

  • The EvictNode command and its atom, and its proposal/voting/commit paths. Command now states its discriminants explicitly and slot 6 is simply absent; crates/p2p/src/conversions/consensus.rs:681 returns "EvictNode command is no longer supported".
  • EvictionProof generation and submission to layer one, and with it the BaseLayerDb / LayerOneSubmitter plumbing.
  • The missed_proposal_evict_threshold consensus constant and the enable_eviction_proposal flag.
  • Leader-view skipping of evicted validators, and with it the vote_to_skip_next NEWVIEW-on-vote path.

From that commit message: "Eviction is disabled on every network ... and is not a mechanism we want to carry into mainnet: it is a consensus-level punishment with no recovery path, gated on a missed-proposal counter that conflates liveness faults with equivocation."

missed_proposal_suspend_threshold (5) and missed_proposal_recovery_threshold (5) are still tracked, but they no longer drive anything in consensus — so the claim that peers "immediately send a NewView to the next leader when the suspended node's turn comes" is no longer true either; that was vote_to_skip_next.

Affected:

File Line What to fix
RFC-0303_DanOverview.md 187 "evicted ... using the EvictNode command"
RFC-0305_Consensus.md 137–151, 221, 243 Whole "Identifying and removing malicious nodes" rewrite; missed_proposal_evict_threshold (10) does not exist
RFC-0313_VNRegistration.md 201–212 "Exit and eviction" — eviction is not one of the exit paths; there is now no path by which L2 consensus writes back to the L1 registry
RFC-0314_VNCSelection.md 185 "eventually evicted"
RFC-0321_ProcessingForeignProposals.md 78, 136 EvictNode in the glossary and in the command ordering
RFC-0330_Cerberus.md 182, 186, 241 EvictNode as a command, and as first in the ordering

Also: both RFC-0330:186 and RFC-0321:135-136 state the block command ordering as "EvictNode first, then ForeignProposal …". CommandOrdering in crates/storage/src/consensus_models/command.rs:119 is ForeignProposal(ShardGroup, &BlockId), TransactionId, EndEpoch — no eviction term. The rest of that sentence is correct as written.

2. epoch_end_spread_blocks does not exist, and the described behaviour is the opposite of what's implemented

  • RFC-0313_VNRegistration.md:132 lists it as an Ootle consensus constant with value 5.
  • RFC-0313_VNRegistration.md:254 and the whole "Spread" section in RFC-0325_DanTimeManagement.md:129-148 describe it as leeway that lets a validator whose scan has not crossed the boundary still accept an EndEpoch proposal from peers whose oracle has.
  • RFC-0325_DanTimeManagement.md:156 puts it into the transition sequence: "Replicas ratify that hash against their own oracle, applying the epoch_end_spread_blocks leeway, and vote."

There is no such field on ConsensusConstants, and no symbol by that name anywhere in the repo. More importantly, crates/consensus/src/hotstuff/on_ready_to_vote_on_local_block.rs:396-421 does the exact opposite — it gates strictly on the local oracle and abstains when the boundary has not been observed:

Ratify the next epoch's hash against our own (lagged, reorg-stable) oracle. Voting for the EOE block is how the committee agrees on the next epoch's boundary hash; by gating on the LOCAL oracle (not on the majority signal that may have set can_propose_epoch_end above), a node never lends quorum to a hash it has not itself observed. If our oracle has not crossed the boundary, abstain until it has.

A node in that position sets NoVoteReason::EndOfEpochHashNotObserved. The liveness argument the "Spread" section makes is real, but the implementation answers it with the scan lag (which makes the race rare) plus deferral, not with a leeway window. RFC-0325:167 — "the transition is deferred and retried when the oracle catches up" — is the accurate description; the preceding section contradicts it.

3. The exhaust burn is a share of the fees collected, not a surcharge on top

RFC-0320_TurbineModel.md:201-204:

When the engine settles a transaction's fees it charges the burn on top of the accrued execution fee, as a separate FeeSource::ExhaustBurn charge. The leader therefore receives the execution fee in full, and the burnt amount is destroyed rather than redistributed.

That was the model, and it was explicitly replaced. crates/engine_types/src/fees.rs:

  • ExhaustBurnRate — "The share of the fees a transaction paid that is burned rather than paid to leaders. A rate reaches consensus only through this type, so no network can be configured to burn more than was collected."
  • FeeReceipt::pre_burn_fees_paid() — "The share of total_fees_paid that flows to leaders: what was collected less the exhaust burn" — literally total_fees_paid().saturating_sub(self.exhaust_burn()).
  • FeeSource slot 9 is now Reserved, with: "Never charged. Slot 9 carried the exhaust burn surcharge before the burn became a share of what was paid, recorded on FeeReceipt::exhaust_burn."

So there is no FeeSource::ExhaustBurn, and the leader receives the fee less the burn. The same claim appears in two more places:

  • RFC-0323_TariThrottle.md:70 — "so leaders receive the execution fee in full and the burnt amount is destroyed separately"
  • RFC-0350_TariVM.md:207 — the ExhaustBurn row in the FeeSource table

The consensus-rule framing and the 500 bps / 5% figure are both correct; it's only the direction of the charge that's wrong.

4. FeeSource::SignatureVerification does not exist

RFC-0350_TariVM.md:205. The full enum is Initial, RuntimeCall, Storage, TransactionWeight, TemplateLoad, SubstateCreate, WasmExecution, TemplatePublish, Reserved (9), NativeExecution — slot 4 is marked "Reserved for future use" and there is no signature-verification source in the tree. Signature-verification work is priced through NativeExecution, which the table already has. Dropping the row (and the ExhaustBurn row from #3) makes the table exactly the enum.

5. base_layer_confirmations on mainnet is 780, not 1000

RFC-0313_VNRegistration.md:131 and RFC-0325_DanTimeManagement.md:115. ConsensusConstants::MAINNET sets base_layer_confirmations: 780 — Minotari's coinbase_min_maturity (720) plus 60 blocks of margin, deliberately a multiple of the L1 vn_epoch_length so the lag ends on an epoch boundary. Esmeralda/testnets at 100 and devnet at 3 are correct.

Related, in the same sentence at RFC-0325:114-115: the oracle "scans at a configured height_lag behind the tip, and a block is only treated as final once base_layer_confirmations blocks sit on top of it" reads as two independent gates. They're one value — applications/tari_validator_node/src/bootstrap.rs:656 and applications/tari_indexer/src/bootstrap.rs:558 both do height_lag: consensus_constants.base_layer_confirmations. The note at RFC-0325:124-127 gets closer but "alongside" still implies two knobs.


Non-blocking

  • RFC-0330_Cerberus.md:236 — "A quorum is $2f+1$ of the committee's vote power". Committee::quorum_threshold computes $n - f$ and documents the distinction: "the quorum threshold $n - f$ ... This equals $2f + 1$ exactly when $n = 3f + 1$." With vote power that isn't generally the case (e.g. $n = 41 \Rightarrow f = 13$, quorum 28, not 27). Worth stating as $n - f$.
  • RFC-0350_TariVM.md:174 — a code span is broken across the line wrap: `PutLastInstructionOutputOn` / `Workspace`. Renders as two spans; join it into `PutLastInstructionOutputOnWorkspace`. The other 14 instruction names on that line match Instruction exactly.
  • RFC-0320_TurbineModel.md:197 — "One µXTM burnt yields one µXTR". XTR is the deprecated ticker (the ootle repo's own guidance is to use TARI); suggest "one µT of Tari" or similar.
  • RFC-0325_DanTimeManagement.md:105-106 — the EpochEvent list omits the Error variant. Trivial, but the list reads as exhaustive.
  • RFC-0303_DanOverview.md:236 and RFC-0320_TurbineModel.md:69 link submarine swaps as "RFC-0310" while every other cross-reference in those documents uses the new labels — should be P-TIP-RFC-O-0310.
  • Orphaned link definitions. [base node] is now unused in RFC-0313, RFC-0314 and RFC-0325, and [contract template] in RFC-0330, after the rewrites dropped the last usage. Harmless, but dead.
  • SUMMARY.md:80,85 — RFC-0323_TariThrottle.md and RFC-0388_BearerTokens.md move into Deprecated while keeping RFC- filenames; every other entry in that section uses RFCD-. Renaming would break inbound links, so leaving them is probably right — just noting the inconsistency in case a follow-up wants to do the rename with redirects.

Spot-checked and correct

For calibration, these new claims all verify against the tree:

  • 256 preshards (NumPreshards::current() == P256); entity-id byte selects the shard via SubstateAddress::to_shard reading the top bits of byte 0.
  • $n_\text{committees} = \min(n_\text{preshards}, \max(1, \lfloor N_\text{vn} / \texttt{committee_size_per_shard_group}\rfloor))$ — calculate_num_committees plus the min(num_shards) clamp in to_shard_group; and the remainder genuinely does spread one preshard at a time over the lowest-numbered groups.
  • committee_size_per_shard_group = 40 on mainnet/Esmeralda/testnets (devnet is 7, correctly excluded).
  • pacemaker_block_time = 10s; leader = height % committee.len() (RoundRobinLeaderStrategy).
  • 36-byte substate address = 32-byte ObjectKey + 4-byte version; all ten SubstateValue variants listed, none invented.
  • Exhaust rate 500 bps on every network, and exhaust_burn_rate(&self, epoch) is indeed epoch-aware.
  • Foreign proposals: notify-then-pull, ForeignProposalNotification / ForeignProposalRequest, statuses New/Proposed/Confirmed/Invalid, ForeignPledgeInputConflict and ForeignShardGroupDecidedToAbort all real; relaxed ordering is what ships.
  • Epoch oracles: base_layer, configured, hybrid; height_lag, scanning_interval, lock_epoch all real.
  • Full 15-instruction set in RFC-0350 matches Instruction exactly.
  • RFC-0388's replacement model (AccessRule / RestrictedAccessRule / RuleRequirement / RequireRule, SubstateOwnerRule) matches tari_template_lib_types::access_rules and owner_rule.
  • RFC-0310's rewrite against Schnorr adaptor signatures and stealth SpendCondition trees with BuiltinPredicate timelocks matches what's in crates/wallet/crypto/src/adaptor.rs and crates/template_lib_types/src/stealth/unspent_output.rs.
  • rpc_state_sync, the per-shard Jellyfish Merkle state tree, tari_ootle_p2p, and tari_indexer_lib vs the tari_indexer application are all named correctly.

I did not verify the base-layer constants table in RFC-0313:110-123 (vn_epoch_length, vn_registration_*) — those live in the tari repo, which I don't have access to here. Worth a second pair of eyes from someone who does, especially since the surrounding text flags the mainnet values as provisional.


Generated by Claude Code

This branch has not been deployed

No deployments
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.

2 participants