fix(nft): report externally signed trades to the Baked Bazaar indexer - #8
Conversation
fibanachos
left a comment
There was a problem hiding this comment.
Ran yarn test on the head (lint, format, typecheck, 658 unit tests, smoke with 57 tools): green. The bazaarLog round trip works the way you describe. The blocker is the merge conflict with main. The other two comments are small.
| `sign transactionBase64 with wallet ${this.publicKey.toBase58()} (do not modify it — it is ` + | ||
| `already verified, simulated and co-signed), then call submit_signed_tx with the signed ` + | ||
| `bytes and the same submit/blockhash/lastValidBlockHeight fields` + | ||
| `bytes and the same submit/blockhash/lastValidBlockHeight` + |
There was a problem hiding this comment.
This conflicts with main: the security fix in 6b0dd22 rewrote this same next string (it now tells the wallet to show the user what the tx does), so the PR can't merge as is. When you rebase, keep main's wording and just add the /bazaarLog bit.
Prompt for an agent
First verify the conflict is real: in cookie-mcp, run `git fetch origin && git merge-tree --write-tree --name-only origin/main HEAD` on branch feat/bazaar-log-external. If it reports no conflicts, stop and report back instead of changing anything.
If it does: rebase feat/bazaar-log-external onto origin/main. In src/core/signer.ts ExternalSigner.signTransaction, keep main's new `next` text (the "any co-signatures would break; it was simulated, and its effect on this wallet checked against the request..." wording) and insert only the conditional `/bazaarLog` before " fields". Resolve README.md by keeping both sides' bullets. Do not change any other wording.
Verify: `yarn test` passes (lint, format, typecheck, unit, smoke = 57 tools), and src/core/signer.test.ts "echoes a marketplace bazaarLog" still matches the `next` regex.
| import bs58 from "bs58"; | ||
|
|
||
| import type { ProvidedSignature } from "./context"; | ||
| import type { BazaarLog } from "./nft/bazaar"; |
There was a problem hiding this comment.
Since you asked about the shape: the approach is fine, but this makes the generic signer protocol import from a venue module (nft/bazaar), and submit.ts does the same. I'd define the payload type in signer.ts (or a neutral types file) and have bazaar.ts import it, so the core seam doesn't depend on one marketplace.
Prompt for an agent
First verify the claim: check that src/core/signer.ts and src/core/submit.ts import `BazaarLog` from "./nft/bazaar", and that nothing else in src/core/signer.ts depends on a venue module. If signer.ts no longer imports from nft/, stop and report back instead of changing code.
If it holds: move the `BazaarLog` interface out of src/core/nft/bazaar.ts into src/core/signer.ts (exported, same name and fields), and make bazaar.ts, nft/index.ts and submit.ts import the type from "../signer" / "./signer". Keep the runtime call to `logTransaction` in submit.ts as is. Do not rename the `bazaarLog` field or change the zod schema in src/mcp/createServer.ts.
Verify with `yarn typecheck && yarn test:unit && yarn lint`.
| messageId = await bridgeMessageId(conn, signature); | ||
| } | ||
| // The NFT tool would have told the indexer after its own confirm; do the same here. | ||
| if (args.bazaarLog && confirmed) { |
There was a problem hiding this comment.
This posts to the Bazaar indexer for any route, so a bazaarLog passed with a solana-rpc submit would report a Solana signature. NFT txs only ever go out on cookie-rpc; I'd only report on that route.
Prompt for an agent
First verify: in src/core/submit.ts submitSignedTransaction, check whether the `logTransaction` call is gated on the route. Also confirm in src/core/nft/index.ts that sendNftTx → signSendConfirm always uses `submit: { via: "cookie-rpc" }` (src/core/liquidity/send.ts). If the report is already restricted to cookie-rpc, or NFT txs can use another route, stop and report back instead of changing code.
If it holds: change the condition to `if (args.bazaarLog && confirmed && route.via === "cookie-rpc")`. Add a test in src/core/submit.test.ts that calls submitSignedTransaction with `submit: { via: "solana-rpc" }` plus a bazaarLog and asserts fetch was not called. Do not change the candyshop branch.
Verify with `yarn test:unit src/core/submit.test.ts` and `yarn test`.
e834b2d to
192af77
Compare
|
Rebased onto main and addressed all three: kept main's |
Every NFT tool calls `logTransaction` after its own send and confirm. With an external signer the
flow stops at the signing step, so those calls never ran and a listing or offer placed through a
hosted deployment stayed invisible until the indexer's own scan found it.
The report now travels with the transaction: the NFT tools pass a `bazaarLog` ({ type, nftMint,
price? }) through `SignContext`, `ExternalSigner` echoes it in `needs_signature`, and
`submit_signed_tx` takes it back and reports it once the transaction confirms. `sendNftTx` makes
the same report for a local signer, so the six call sites no longer repeat it.
`BazaarLog` moves to `signer.ts`, so the generic signer seam and `submit.ts` no longer import a type from a venue module; `nft/bazaar.ts` imports it instead. `submit_signed_tx` reports to the indexer only for a transaction sent on the Cookie Chain RPC — the one route NFT transactions use — so a `bazaarLog` passed with a Solana submit cannot report a Solana signature.
192af77 to
0a677eb
Compare
|
LGTM |
Since 0.5.0, NFT trades signed through the external signer are never reported to Baked Bazaar's indexer. Each NFT tool calls
logTransactionafter its own send and confirm, and in external mode the flow stops at the signing step before reaching it.submit_signed_txfinishes the send, but it has no idea the transaction was a marketplace trade. So a listing or offer placed through a hosted deployment stays invisible on bakedbazaar.art until the indexer's own scan picks it up.This PR is a proposal for the shape. Happy to redo it another way if you prefer.
What it does
BazaarLog({ type, nftMint, price? }) is the payload the six call sites already send, now typed inbazaar.ts.SignContext.bazaarLog: the NFT tools pass it throughsignSendConfirm, andExternalSignerechoes it inneeds_signature.nextthen asks for it back.submit_signed_txacceptsbazaarLog(zod-validated) and reports it with the signature only once the transaction confirms. That is the same point where the local path reports it.sendNftTxmakes the report for a local signer, so the six call sites no longer repeat it. The behaviour in local mode is unchanged.Alternatives I considered
submit. No new field and nothing for the caller to carry. Butpublic_buy/canceldo not name the mint, so it would need account lookups and a decoder per instruction.afterConfirmhook instead of a Bazaar-specific field. It is more general, but there is only one user of it today.The caller could POST a made-up
bazaarLogthroughsubmit_signed_tx. They can already POST the same thing to the public/log-transactionendpoint directly, so this adds nothing new.Checked
COOKIE_SIGNER=external,makeOffer(with fix(nft): make_offer fails with 3012 when the bidder has no token account #7 applied, since it fails with 3012 without it) →needs_signaturecarriesbazaarLog: {"type":"offer","nftMint":"6P8pcgau…","price":"10000000"}for a 0.01 COOK offer.ExternalSignerechoesbazaarLog.submitSignedTransactionPOSTs{ signature, ...bazaarLog }to/log-transactionafter the transaction confirms, and posts nothing when confirmation fails or no log was passed. The first test fails if the report insubmit.tsis removed.yarn lint,format:check,typecheck,smokepass;test:unit656 pass (the twoimageFile.test.tsfailures are onmaintoo, Windows-only).CHANGELOG.mdconflict (both append to Unreleased → Fixed). I'll rebase whichever lands second.