Per rollforsats feedback on PR #226 msg 82: the Status field on
BtcMapsBtcMapResult was a hardcoded 'submitted-for-review' constant
that didn't reflect anything the btcmap RPC actually returned.
submit_place's result envelope contains id / origin / external_id;
the queued-for-reviewer-workflow semantics are documented on the
BTC Map side, not signalled per-response. Carrying it on every
response was misleading.
Per CREATOR direction on PR #226. Tightening the existing endpoint-wide
fixed-window from 5/24h to 3/24h per source IP. The BTC Map import-RPC
lane forwards submissions into the upstream reviewer queue (not an
instant publish), and rate-limit is the primary spam control on the
public endpoint.
Per rollforsats msg 76 + Hermes finding #4 on PR #226. payment:onchain
and payment:lightning are OSM-style custom tags, so they follow the
osm:<tag_name> convention btcmap documents for additional tags in
extra_fields. Emit as osm:payment:onchain / osm:payment:lightning.
Per Hermes finding #5 on PR #226. BTC Map's import RPC routes
submitted places into a reviewer queue rather than instant publish.
Successful submit_place means 'submission accepted for review', not
'place live on the directory map.'
Add a Status field on BtcMapsBtcMapResult defaulting to
'submitted-for-review' so clients reading the response shape get
the workflow semantics by default. Constant value clients can key
off if they want to surface the queued state in UX.
Per Hermes review findings #2 and #3 on PR #226.
#2 - Url / Description / Type were required unconditionally. BTC Map
submit_place needs none of those (its required-fields are origin +
external_id + lat + lon + category + name). Wrap the directory-lane
required-field checks in if(request.SubmitToDirectory) so a caller
with SubmitToBtcMap=true + SubmitToDirectory=false doesn't have to
populate unrelated directory metadata.
Name validation stays unconditional - both lanes need it.
#3 - Country=GLOBAL is the directory's pseudonym for online-only /
multi-region merchants; it has no meaning for the btcmap directory
map (every place is physically geocoded). Reject Country=GLOBAL when
SubmitToBtcMap=true so we don't forward osm:addr:country=GLOBAL to
btcmap. Directory-only callers keep the existing GLOBAL semantics.
Per rollforsats msg 73 on PR #226. The comment narrated decision
history from an earlier review pass that subsequent iterations
partially reverted, so the running narrative is now misleading.
Keep the load-bearing JSON-RPC envelope reference, drop the rest.
Per rollforsats msg 71 on PR #226. Iterating on extra_fields shape:
- Re-add osm:addr:country mapping (existing Country field, now using the
granular osm:addr:* prefix in line with the other address fields).
- Re-add twitter mapping, but using the first-class `twitter` URL key
per btcmap rest/v4/places.md (not contact:twitter). Normalize @handle
vs URL input shapes.
- Add Email first-class field (`email` key in btcmap doc).
- Replace always-emit payment:onchain + payment:lightning with caller-
controlled bool flags AcceptsOnchain + AcceptsLightning. Each emits
the corresponding payment:<rail>=yes marker only when explicitly true.
Lightning-only or on-chain-only stores can now signal accurately.
Per rollforsats feedback on PR #226: trim the extra_fields surface to
what's directly useful for the btcmap directory map, and add granular
address tags + per-rail payment markers.
Drops:
- contact:twitter / contact:github / contact:onion - these channels
aren't surfaced on the btcmap directory map; the rollforsats plugin
side can iterate later if a use case surfaces.
- addr:country - replaced by the granular osm:addr:* fields below.
Bare "addr:" prefix is not the btcmap-documented convention; their
doc specifies osm:<tag_name> for OSM-style tags.
- payment:bitcoin=yes - replaced by the per-rail markers below; one
yes/no is less informative than telling the directory which rails
the merchant accepts.
Adds:
- HouseNumber, Street, City, Postcode fields on BtcMapsSubmitRequest;
plugin captures these alongside lat/lon. Forwarded as
osm:addr:housenumber / osm:addr:street / osm:addr:city /
osm:addr:postcode per the btcmap rest/v4/places.md osm:<tag_name>
convention (https://github.com/teambtcmap/btcmap-api/blob/master/docs/rest/v4/places.md).
- payment:onchain=yes + payment:lightning=yes always emitted. BTCPay
merchants run both rails by default; granular per-rail flags can
follow if a store-level disable becomes useful.
Keeps: website, description, phone.
Per CodeRabbit review on PR #226. A misconfigured
BTCMAPS:BtcMapImportEndpoint over http:// would silently leak the
scoped token to anyone on the network path between plugin-builder
and btcmap. Parse the configured value as an absolute https URI
before building the request, throwing InvalidOperationException
with the offending value if the parse / scheme check fails. The
exception fires before SendAsync, so the token never reaches a
HttpRequestMessage header.
Adds 3 tests: http-rejected, non-absolute-rejected, token-missing
maps to BtcMapTokenMissingException (controller-ladder regression
guard). 40/40 BtcMapsServiceTests passing.
Adds a second downstream lane to /apis/btcmaps/v1/submit that forwards
the merchant payload to teambtcmap/btcmap-api's submit_place RPC
(merged 2026-05-24 in teambtcmap/btcmap-api#91).
Request schema:
- New fields Lat, Lon, Category, ExternalId on BtcMapsSubmitRequest,
required iff SubmitToBtcMap=true. Validator enforces lat/lon
ranges, lowercase-identifier category, and 1-200 char external_id.
Plugin side (rollforsats/BTCPayServerPlugins) composes external_id
as hostname:storeId so the namespace stays unique per BTCPay
instance.
- New SubmitToDirectory + SubmitToBtcMap routing flags. The
directory flag defaults true to preserve existing callers; btcmap
defaults false so new callers must opt in.
- New Phone field forwarded as OSM Key:phone in extra_fields.
Service layer:
- BtcMapsService.SubmitToBtcMapAsync POSTs a JSON-RPC 2.0 envelope
({jsonrpc, method, params, id}) to BTCMAPS:BtcMapImportEndpoint
(default https://api.btcmap.org/rpc) with method=submit_place,
origin=btcpayserver, and the merchant payload mapped to the
documented param shape. Bearer auth from BTCMAPS:BtcMapImportToken.
- Optional fields (website, description, twitter, github, onion,
phone, country) ride along in extra_fields using OSM tag keys
(contact:twitter, addr:country, etc.) plus the implicit
payment:bitcoin=yes marker.
- New BtcMapTokenMissingException parallels the existing
DirectoryTokenMissingException so the controller can return 503
with a distinct error code when ops haven't provisioned the
scoped token yet.
Controller:
- /apis/btcmaps/v1/submit branches on SubmitToDirectory +
SubmitToBtcMap. At least one must be true (rejected 400 otherwise).
- Each lane has its own exception ladder symmetric to the existing
directory path: token-missing 503 (directory-not-configured /
btcmap-not-configured), caller-cancel rethrow, upstream-timeout
504, generic-failure 502 - error codes namespaced by lane so ops
can tell them apart.
HttpClient registration:
- New HttpClientNames.BtcMap named client registered with 15s
per-call timeout and JSON Accept header, matching the
BtcMapsDirectory budget for bounded worst-case behavior.
Tests:
- 12 new validation tests in BtcMapsServiceTests covering the
SubmitToBtcMap=true required-field paths (Lat / Lon / Category /
ExternalId; range checks; lowercase-identifier policy; overlong
external_id) plus the default-false directory-only-still-works
baseline. 37/37 BtcMapsServiceTests passing.
Invariant scaffolding for the btcmap import-RPC integration. Both
changes are additive (Phone is optional, BtcMap constant unreferenced
until impl). Build clean.
Pending coordination Qs from rollforsats (lat/lon source, category
mapping, external_id namespace) before service + controller wiring.
Addresses thgO-O review id 4274558448 (PR #224) items #2 and #3.
#2 OperationCanceledException vs HttpClient.Timeout: the prior
catch-when filter excluded OCE wholesale to avoid swallowing client
disconnects, but HttpClient.Timeout surfaces as TaskCanceledException
(inherits from OCE) so an upstream GitHub timeout slipped through and
fell out as a 500 instead of the intended 502. Now split into three
arms: caller-cancel rethrows (info-log + drop the connection),
upstream-timeout returns 504 with discriminant directory-upstream-
timeout, anything else still 502 directory-upstream-failed.
#3 type/subType case normalization on write: ValidTypes +
ValidMerchantSubTypes match OrdinalIgnoreCase at validation, but the
write at BuildMerchantEntry preserved the user-submitted casing. A
submission of "Merchants" / "Books" would pass validation and land
non-canonical in merchants.json. Normalize to ToLowerInvariant on
write so the file stays in the lowercase convention. Country is
already validated against a case-sensitive Ordinal set so the
validator rejects non-uppercase before reaching the writer; no write-
path change needed there.
Addresses post-#224 review feedback from @rollforsats + CodeRabbit:
- IHttpClientFactory + named HttpClientNames.BtcMapsDirectory client
replaces per-request `new HttpClient()`. 15s per-call timeout caps the
~5-7 GitHub round-trips at a bounded worst case instead of the default
100s x N. Bearer token stays per-request (the BTCMAPS token is distinct
from the global PluginBuilder GitHub token; must not leak into the
singleton handler).
- Markdown injection guard on the PR body. User fields (Name, Type,
SubType, Country, Twitter, GitHub) are wrapped in inline code spans
with backtick-escape so a doctored merchant name can't render as a
clickable link in the maintainer-facing PR description. Description
goes inside a fenced code block. URL is rendered as <bare-url> autolink
so the maintainer always sees the actual destination.
- Idempotent branch name: SHA-1-derived suffix from the normalized URL
replaces the random GUID. Two concurrent same-URL submissions now
collide on `git/refs` create instead of racing through preflight and
opening duplicate PRs. The 422 "Reference already exists" surface is
caught and mapped to the open-PR lookup or `branch-exists-no-open-pr`.
- NormalizeUrl lowercases scheme + host only and preserves path + query
case verbatim. Lowercasing the whole URL falsely de-duplicates
case-sensitive paths.
- Country code validation moves to an actual ISO 3166-1 alpha-2 set
built from CultureInfo at startup. Replaces the
`length==2 && IsUpper` shape that accepted reserved/unassigned codes
like ZZ.
- Missing BTCMAPS:DirectoryGithubToken throws
DirectoryTokenMissingException at the service layer; controller maps
it to 503 with `directory-not-configured`. Previously surfaced as a
200 OK with `Skipped` which a client could misread as "accepted".
5 new tests:
- Validate_RejectsNonAssignedTwoLetterCountry (ZZ)
- NormalizeUrl_PreservesPathCase
- NormalizeUrl_PreservesQueryCase
- BuildBranchName_DeterministicForSameUrl
- BuildBranchName_DiffersForDifferentUrls
25/25 BtcMapsServiceTests pass on Release build.
Supersedes PR #211. Per-store OSM OAuth moves to the BTC Map
plugin side (rollforsats/BTCPayServerPlugins PR #5); the
plugin-builder side keeps only the directory PR submission.
Drops vs PR #211:
- TagOnOsmAsync / UnlistFromOsmAsync service paths and all
OSM XML + changeset infrastructure (~430 lines)
- TagOnOsm / UnlistFromOsm / OsmNodeId / OsmNodeType /
Latitude / Longitude / OsmCategory / AcceptsLightning
request fields
- BtcMapsOsmResult response shape + Address sub-model
- OSM-specific validators
Keeps:
- POST /apis/btcmaps/v1/submit opens a PR against
btcpayserver/directory.btcpayserver.org's merchants.json
- GET /apis/btcmaps/v1/ping
- Rate limit: 5 submissions / 24h per source IP
- Validation for name / url / description / type / subType /
country / twitter / github / onionUrl
Build clean (Release); 20 unit tests cover validation, slug,
URL normalization.
Add an explicit local artifact download proxy flag for development and tests.
Keep the public download endpoint as a redirect while routing enabled loopback artifacts through the internal proxy.
