StarkColumnsView<T> structs for other STARKs

[gio256]

Currently the CpuStark and KeccakSpongeStark use structs to represent their respective columns, while other starks use raw indices into their columns. Is there a reason for this difference, or would some PRs refactoring other starks to use StarkColumnsView<T> structs be helpful?

The CpuColumnsView struct:

zk_evm/evm_arithmetization/src/cpu/columns/mod.rs

Lines 58 to 95 in baecefc

	pub(crate) struct CpuColumnsView<T: Copy> {
	/// If CPU cycle: Current context.
	pub context: T,
	/// If CPU cycle: Context for code memory channel.
	pub code_context: T,
	/// If CPU cycle: The program counter for the current instruction.
	pub program_counter: T,
	/// If CPU cycle: The stack length.
	pub stack_len: T,
	/// If CPU cycle: We're in kernel (privileged) mode.
	pub is_kernel_mode: T,
	/// If CPU cycle: Gas counter.
	pub gas: T,
	/// If CPU cycle: flags for EVM instructions (a few cannot be shared; see
	/// the comments in `OpsColumnsView`).
	pub op: OpsColumnsView<T>,
	/// If CPU cycle: the opcode, broken up into bits in little-endian order.
	pub opcode_bits: [T; 8],
	/// Columns shared by various operations.
	pub(crate) general: CpuGeneralColumnsView<T>,
	/// CPU clock.
	pub(crate) clock: T,
	/// Memory bus channels in the CPU.
	/// Full channels are comprised of 13 columns.
	pub mem_channels: [MemoryChannelView<T>; NUM_GP_CHANNELS],
	/// Partial channel is only comprised of 5 columns.
	pub(crate) partial_channel: PartialMemoryChannelView<T>,
	}

compared to the columns defined in the MemoryStark:

zk_evm/evm_arithmetization/src/memory/columns.rs

Lines 5 to 21 in baecefc

	// Columns for memory operations, ordered by (addr, timestamp).
	/// 1 if this is an actual memory operation, or 0 if it's a padding row.
	pub(crate) const FILTER: usize = 0;
	/// Each memory operation is associated to a unique timestamp.
	/// For a given memory operation `op_i`, its timestamp is computed as `C * N +
	/// i` where `C` is the CPU clock at that time, `N` is the number of general
	/// memory channels, and `i` is the index of the memory channel at which the
	/// memory operation is performed.
	pub(crate) const TIMESTAMP: usize = FILTER + 1;
	/// 1 if this is a read operation, 0 if it is a write one.
	pub(crate) const IS_READ: usize = TIMESTAMP + 1;
	/// The execution context of this address.
	pub(crate) const ADDR_CONTEXT: usize = IS_READ + 1;
	/// The segment section of this address.
	pub(crate) const ADDR_SEGMENT: usize = ADDR_CONTEXT + 1;
	/// The virtual address within the given context and segment.
	pub(crate) const ADDR_VIRTUAL: usize = ADDR_SEGMENT + 1;

[Nashtare]

That's indeed something that's been bugging me for a while but I never got to get around it. The view approach is cleaner IMO, although it may require some workarounds for clean column index access (typically for range-check) as visible on the KeccakSpongeStark side. There's also a bit of code smell for all the associated unwrap() when converting the views to slices but these are pretty harmless given the sizes are known at compile time.

[gio256]

Valida uses a derive macro to implement Borrow and BorrowMut for their column views. Would something similar for the boilerplate column view impls be convenient? I have mixed feelings about hiding unsafe code in proc macros, but it could make things a bit cleaner.

[gio256]

Similar to #311, this would require a standalone sub-crate for proc macros.

[Nashtare]

I have mixed feelings about hiding unsafe code in proc macros

In general I'd also tend to advise against, but in this particular scenario of StarkColumnsView we have safety guarantees when calling transmute() so this seems fine to me. And indeed it may make things cleaner, but no strong opinion, I'm not too bothered by the current way of implementing Borrow manually.

[Nashtare]

Remains:

• ArithmeticStark
• KeccakStark

I haven't had time to handle them yet. Feel free to pick them up, or I'll do them sometime in the coming weeks.