-
Notifications
You must be signed in to change notification settings - Fork 33
handle multiple auctions TGLD burn #1247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@CodeRabbit summary |
✅ Actions performedSummary regeneration triggered. |
📝 WalkthroughWalkthroughThis PR adds a new SENA address field to TEMPLE_GOLD.AUCTIONS.BID_FOR_SPICE across network configs and updates the ContractAddresses type. It introduces getSpiceAuctions(contracts) to extract valid auction addresses and changes task signatures/config to pass auctions[] instead of a single auction. The spice-auction burn-and-notify task and main-localtest setup are refactored to iterate over auctions[], performing per-auction approval, epoch inspection, burnAndNotify transactions, and per-auction gas handling and logging. Sequence Diagram(s)sequenceDiagram
participant TaskRunner as Task Runner
participant Config as Contract Config
participant Helper as getSpiceAuctions
participant BurnFlow as Burn & Notify Flow
participant Auction as Spice Auction Contract
participant TempleGold as TempleGold Contract
TaskRunner->>Config: read TEMPLE_GOLD.AUCTIONS.BID_FOR_SPICE
Config-->>Helper: provide BID_FOR_SPICE object
Helper->>Helper: filter valid addresses (isAddress)
Helper-->>TaskRunner: return auctions: Address[]
TaskRunner->>BurnFlow: invoke with auctions[]
loop for each auctionAddress
BurnFlow->>Auction: instantiate contract at auctionAddress
BurnFlow->>Auction: approveMaxTgld(auction)
Auction-->>BurnFlow: approval confirmed
BurnFlow->>Auction: read currentEpoch & epochInfo
Auction-->>BurnFlow: epoch data
BurnFlow->>BurnFlow: gather unnotified epochs (per-auction)
BurnFlow-->>Auction: send burnAndNotify(tx) (with per-auction gas override if needed)
Auction-->>BurnFlow: tx confirmed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
@CodeRabbit summary |
✅ Actions performedSummary regeneration triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/automation/automation-v2/src/main-localtest/mainnet.ts`:
- Around line 273-277: The code currently reads the daoExecutor from the
hardcoded config.contracts.TEMPLE_GOLD.AUCTIONS.BID_FOR_SPICE.ENA which can be
the placeholder '0x' in some environments; update the logic in the block that
sets executorAddress (the tclient.readContract call) to first attempt using
config value but fall back to the first valid auction address returned by
getSpiceAuctions() (or filter out invalid '0x' entries) and use that address for
the readContract call so spiceAuctionTestSetup can run against anvil/local
configs.
🧹 Nitpick comments (4)
apps/automation/automation-v2/src/tasks/index.ts (1)
172-175: Useconstinstead ofletfor immutable variable.The
auctionsvariable is never reassigned, soconstis more appropriate.Suggested fix
export function getSpiceAuctions(contracts: ContractAddresses): Address[] { - let auctions = Object.values(contracts.TEMPLE_GOLD.AUCTIONS.BID_FOR_SPICE) as Address[]; + const auctions = Object.values(contracts.TEMPLE_GOLD.AUCTIONS.BID_FOR_SPICE) as Address[]; return auctions.filter((auction) => isAddress(auction)); }apps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts (2)
26-28: Rename type aliases to avoid shadowing imported namespaces.The type aliases
AuctionBaseandTempleGoldshadow the namespace imports of the same names. While TypeScript separates type and value bindings (soAuctionBase.ABIstill resolves to the namespace), this creates confusion and triggers static analysis warnings.Suggested fix
-type Auction = GetContractReturnType<typeof Spice.ABI, PublicClient>; -type AuctionBase = GetContractReturnType<typeof AuctionBase.ABI, PublicClient>; -type TempleGold = GetContractReturnType<typeof TempleGold.ABI, PublicClient>; +type SpiceAuctionContract = GetContractReturnType<typeof Spice.ABI, PublicClient>; +type AuctionBaseContract = GetContractReturnType<typeof AuctionBase.ABI, PublicClient>; +type TempleGoldContract = GetContractReturnType<typeof TempleGold.ABI, PublicClient>;Then update all usages of these types throughout the file (e.g.,
approveMaxTgld(auction: SpiceAuctionContract),getTotalBidTokenAmount(auctionBase: AuctionBaseContract, ...), etc.).
114-170: Consider adding per-auction error handling to prevent one failure from blocking others.Currently, if
burnAndNotifyfails for one auction, the entire task fails and subsequent auctions won't be processed. Depending on operational requirements, you may want to catch errors per-auction, log them, and continue processing remaining auctions.Optional: wrap per-auction processing in try-catch
for (const auctionAddress of params.contracts.auctions) { + try { const auction = getContract({ address: auctionAddress, abi: Spice.ABI, client: pclient }); // ... rest of auction processing ... + } catch (error) { + ctx.logger.error(`Failed to process auction ${auctionAddress}: ${error}`); + // Optionally: continue to next auction instead of failing task + } }apps/automation/automation-v2/src/main-localtest/mainnet.ts (1)
284-294: Consider per-auction error handlingIf
setOperatorfails for one auction, the remaining auctions won't be processed. For test setup this may be acceptable, but you might want to wrap each iteration in a try-catch to ensure all auctions are attempted and failures are logged.Optional: Add per-iteration error handling
for (const auctionAddress of auctionAddresses) { + try { const hash = await tclient.writeContract({ abi: ISpiceAuction.ABI, address: auctionAddress, functionName: 'setOperator', args: [tclient.account.address], account: executorAddress, }); const txReceipt = await tclient.waitForTransactionReceipt({ hash }); console.log(txReceipt); + } catch (e) { + ctx.logger.error(`Failed to set operator for auction ${auctionAddress}: ${e}`); + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/automation/automation-v2/src/config/contract_addresses/anvil.tsapps/automation/automation-v2/src/config/contract_addresses/arbsepolia.tsapps/automation/automation-v2/src/config/contract_addresses/mainnet.tsapps/automation/automation-v2/src/config/contract_addresses/sepolia.tsapps/automation/automation-v2/src/config/contract_addresses/types.tsapps/automation/automation-v2/src/main-localtest/mainnet.tsapps/automation/automation-v2/src/tasks/index.tsapps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-30T23:20:17.523Z
Learnt from: frontier159
Repo: TempleDAO/temple PR: 1214
File: protocol/scripts/deploys/mainnet/team-payments/deploy.ts:9-9
Timestamp: 2025-08-30T23:20:17.523Z
Learning: In Temple DAO's protocol/scripts/deploys/mainnet/team-payments/deploy.ts, the user prefers to keep hard-coded imports for epoch snapshot JSON files (like epoch29b.json) rather than parametrizing them. Avoid suggesting refactoring to make snapshot files runtime parameters.
Applied to files:
apps/automation/automation-v2/src/tasks/index.tsapps/automation/automation-v2/src/main-localtest/mainnet.ts
📚 Learning: 2026-01-13T14:59:55.049Z
Learnt from: marshall2112
Repo: TempleDAO/temple PR: 1224
File: apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/MyActivity/BidsForSpice/hooks/use-myActivity-bidsSpiceHistory.ts:133-136
Timestamp: 2026-01-13T14:59:55.049Z
Learning: In apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/MyActivity/BidsForSpice/hooks/use-myActivity-bidsSpiceHistory.ts, the useEffect that calls fetchData intentionally only depends on wallet, not on fetchData, allAuctions, or getClaimableAtEpoch. This is by design to refetch only when the wallet changes, not when auction data updates.
Applied to files:
apps/automation/automation-v2/src/tasks/index.tsapps/automation/automation-v2/src/main-localtest/mainnet.tsapps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts
📚 Learning: 2025-12-12T00:37:50.665Z
Learnt from: marshall2112
Repo: TempleDAO/temple PR: 1224
File: apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Details/Details.tsx:89-105
Timestamp: 2025-12-12T00:37:50.665Z
Learning: In apps/dapp/src/components/Pages/Core/DappPages/SpiceBazaar/Spend/Details/Details.tsx, timestamp fields auctionStartTime and auctionEndTime are already in milliseconds from the provider, while nextAuctionStartTimestamp is in seconds and must be converted with * 1000 at the call site. Treating timestamp value 0 as invalid/unset is correct domain behavior for auction timestamps.
Applied to files:
apps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts
🧬 Code graph analysis (2)
apps/automation/automation-v2/src/tasks/index.ts (1)
apps/automation/automation-v2/src/config/contract_addresses/types.ts (1)
ContractAddresses(3-34)
apps/automation/automation-v2/src/main-localtest/mainnet.ts (1)
apps/automation/automation-v2/src/tasks/index.ts (1)
getSpiceAuctions(172-175)
🪛 Biome (2.1.2)
apps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts
[error] 27-27: Shouldn't redeclare 'AuctionBase'. Consider to delete it or rename it.
'AuctionBase' is defined here:
(lint/suspicious/noRedeclare)
[error] 28-28: Shouldn't redeclare 'TempleGold'. Consider to delete it or rename it.
'TempleGold' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (11)
apps/automation/automation-v2/src/tasks/index.ts (1)
146-147: LGTM - multi-auction integration looks correct.Both Sepolia and mainnet burn tasks now properly use
getSpiceAuctionsto extract valid auction addresses, enabling the multi-auction burn flow.Also applies to: 161-162
apps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts (2)
57-71: LGTM - approval logic correctly parameterized per-auction.The
approveMaxTgldfunction now properly accepts an auction context and approves spending for that specific auction address when allowance is zero.
78-107: LGTM - epoch gathering logic correctly parameterized.The
gatherAllUnnotifiedEpochsfunction properly accepts auction and auctionBase contexts, enabling per-auction epoch processing with appropriate logging.apps/automation/automation-v2/src/config/contract_addresses/types.ts (1)
19-23: LGTM - type extension for SENA auction support.The addition of
SENA: Addressproperly extends theBID_FOR_SPICEtype to support the new auction.apps/automation/automation-v2/src/config/contract_addresses/sepolia.ts (1)
19-23: LGTM - placeholder value will be correctly filtered.The
'0x'placeholder forSENA(andENA) will be correctly filtered out byisAddressingetSpiceAuctions, so only the validDAIauction address will be processed on Sepolia.apps/automation/automation-v2/src/config/contract_addresses/arbsepolia.ts (1)
19-23: LGTM - placeholder configuration for arbsepolia.All
BID_FOR_SPICEaddresses are placeholders ('0x'), which will result in an empty array fromgetSpiceAuctions. The burn task will iterate over an empty array and complete successfully (no-op until auctions are deployed).apps/automation/automation-v2/src/config/contract_addresses/anvil.ts (1)
19-23: LGTM - Consistent placeholder patternThe
SENAfield with placeholder'0x'follows the same pattern used in other test/dev environments. ThegetSpiceAuctionshelper filters out invalid addresses viaisAddress, so this won't cause runtime issues.apps/automation/automation-v2/src/config/contract_addresses/mainnet.ts (1)
22-22: LGTM - New SENA auction addressThe SENA auction contract address is added with valid format. This enables the multi-auction burn functionality for mainnet.
apps/automation/automation-v2/src/main-localtest/mainnet.ts (3)
14-14: LGTM - Import additionThe
getSpiceAuctionsimport is correctly added to support the multi-auction iteration pattern introduced in this PR.
49-49: LGTM - Gas price configurationNew
burn_tgld_max_gas_priceconfig variable added with reasonable default for local testing.
430-443: LGTM - Multi-auction burn integrationThe
burnTempleGoldfunction correctly passes the filtered auction addresses viagetSpiceAuctionsand uses the new dedicatedburn_tgld_max_gas_priceconfiguration variable. ThegetSpiceAuctionshelper ensures only valid addresses (filtering out'0x'placeholders) are processed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
apps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts
Outdated
Show resolved
Hide resolved
apps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/automation/automation-v2/src/tasks/spice-auction-burn-and-notify.ts`:
- Around line 128-129: The log and sort are wrong: replace ctx.logger.info using
unnotifiedEpochs.keys.length with unnotifiedEpochs.size, and fix the BigInt
comparator in Array.from(unnotifiedEpochs.keys()).sort by using an ordinal
comparator that works for BigInt (e.g. (a,b) => a < b ? -1 : a > b ? 1 : 0);
also ensure epochId is declared (e.g. let epochId) when iterating. Target
symbols: ctx.logger.info, unnotifiedEpochs,
Array.from(unnotifiedEpochs.keys()).sort, and epochId.
- Around line 10-33: The file declares an imported namespace `TempleGold` and
also a type alias `type TempleGold = GetContractReturnType<typeof
TempleGold.ABI, PublicClient>;`, causing a duplicate identifier; rename the type
alias (for example to `TempleGoldContract` or `TempleGoldInstance`) and update
every use of that type alias (references to `TempleGold` as a type, e.g., where
`Auction` and the former `TempleGold` type are used) so the import namespace
`TempleGold` remains unchanged and TypeScript compiles.
Description
What does this PR solve?
Fixes # (issue)
Checklist
Summary by CodeRabbit
New Features
Configuration
Behavioral Changes
✏️ Tip: You can customize this high-level summary in your review settings.