Carry auction id in BeginSettle instruction data#79
Conversation
fedgiac
left a comment
There was a problem hiding this comment.
I'm fine with the change, just some comments.
| /// A sample auction id whose little-endian bytes are visually distinct, so | ||
| /// the wire-layout assertions read clearly. | ||
| const AUCTION_ID: i64 = 0x0102_0304_0506_0708; | ||
|
|
There was a problem hiding this comment.
Considering that these tests are about decoding and encoding, I wouldn't use a global constant for all tests but, in the cases where it makes sense, just spell out the actual bytes and only use the placeholder where it doesn't matter.
| pub finalize_ix_index: u16, | ||
| /// The off-chain auction this settlement executes, carried so it can be tied | ||
| /// back to its auction off-chain. | ||
| pub auction_id: i64, |
There was a problem hiding this comment.
Why i64 instead of u64? Will there ever be a time when we want auction ids to be negative?
There was a problem hiding this comment.
I suspect this regards PostgreSQL only supporting signed integers, and I believe we can relax it to an u64 (and deal with signedness validation at the BE side)
There was a problem hiding this comment.
I won't push much for using u64 if this makes the back-end more complex, in the end this isn't really used by the contract. I just want to be sure this is an intentional decision.
There was a problem hiding this comment.
Yeah, this is intentional. The auction id round-trips from the backend, through the instruction and the indexer, and back to the backend, where it's stored as Postgres BIGINT, which is i64 (same as the EVM autopilot's auction_id). The contract never reads it, so matching the backend type end to end avoids an i64/u64 conversion and a signedness check at the boundary, for a value that's never negative in practice. Happy to switch to u64 if we'd rather enforce non-negativity in the type, but that just moves the conversion to the backend for no on-chain gain.
…settlement test helper
Description
A settlement transaction carries no way to tell which off-chain auction it executed, and something off-chain needs to tie the two together. EVM already solves this: the driver appends the auction id to the settle calldata, the contract ignores the trailing bytes, and the off-chain side reads it back off the transaction input.
This puts an
auction_id(i64, little-endian) in theBeginSettleinstruction data, between the counterpart index and the per-order pull layout. The program never reads it. It is carried only so a settlement can be tied back to its auction off-chain.FinalizeSettlelinks back toBeginSettlethrough its counterpart index, so it carries no copy of its own.Changes
BeginSettlebuilder andBeginSettleInputparser carry anauction_id: i64, little-endian, right afterfinalize_ix_index.rejects_missing_auction_idtest.BeginSettlewrapper passes the field through.How to test
Existing tests