Validate that a registrar has billing accounts for all its allowed TLDs (#1601)

This will require edits to a substantial number of registrars on sandbox (nearly
all of them) because almost all of them have access to at least one TLD, but
almost none of them have any billing accounts set. Until this is set, any updates
to the existing registrars that aren't adding the billing accounts will cause
failures.

Unfortunately, there wasn't any less invasive foolproof way to implement this
change, and we already had one attempt to implement it on create registrar
command that wasn't working (because allowed TLDs tend not to be added on
initial registrar creation, but rather, afterwards as an update).

core/src/main/java/google/registry/model/OteAccountBuilder.java

@@ -38,6 +38,7 @@ import google.registry.model.registrar.RegistrarAddress;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.model.tld.Registry;
 import google.registry.model.tld.Registry.TldState;
+import google.registry.model.tld.Registry.TldType;
 import google.registry.model.tld.label.PremiumList;
 import google.registry.model.tld.label.PremiumListDao;
 import google.registry.persistence.VKey;
@@ -111,6 +112,17 @@ public final class OteAccountBuilder {
           DateTime.parse("2030-03-01T00:00:00Z"),
           Money.of(CurrencyUnit.USD, 0));
 
+  /**
+   * The default billing account map applied to all OT&E registrars.
+   *
+   * <p>This contains dummy values for USD and JPY so that OT&amp;E registrars can be granted access
+   * to all existing TLDs in sandbox. Note that OT&amp;E is only on sandbox and thus these dummy
+   * values will never be used in production (the only environment where real invoicing takes
+   * place).
+   */
+  public static final ImmutableMap<CurrencyUnit, String> DEFAULT_BILLING_ACCOUNT_MAP =
+      ImmutableMap.of(CurrencyUnit.USD, "123", CurrencyUnit.JPY, "456");
+
   private final ImmutableMap<String, String> registrarIdToTld;
   private final Registry sunriseTld;
   private final Registry gaTld;
@@ -305,6 +317,7 @@ public final class OteAccountBuilder {
             .setTldStateTransitions(ImmutableSortedMap.of(START_OF_TIME, initialTldState))
             .setDnsWriters(ImmutableSet.of("VoidDnsWriter"))
             .setPremiumList(premiumList.get())
+            .setTldType(TldType.TEST)
             .setRoidSuffix(
                 String.format(
                     "%S%X",
@@ -334,6 +347,7 @@ public final class OteAccountBuilder {
         .setPhoneNumber("+1.2125550100")
         .setIcannReferralEmail("nightmare@registrar.test")
         .setState(Registrar.State.ACTIVE)
+        .setBillingAccountMap(DEFAULT_BILLING_ACCOUNT_MAP)
         .build();
   }
 

core/src/main/java/google/registry/model/registrar/Registrar.java

@@ -81,6 +81,7 @@ import google.registry.model.common.EntityGroupRoot;
 import google.registry.model.registrar.Registrar.BillingAccountEntry.CurrencyMapper;
 import google.registry.model.replay.DatastoreAndSqlEntity;
 import google.registry.model.tld.Registry;
+import google.registry.model.tld.Registry.TldType;
 import google.registry.persistence.VKey;
 import google.registry.util.CidrAddressBlock;
 import java.security.cert.CertificateParsingException;
@@ -798,13 +799,9 @@ public class Registrar extends ImmutableObject
     }
 
     public Builder setBillingAccountMap(@Nullable Map<CurrencyUnit, String> billingAccountMap) {
-      if (billingAccountMap == null) {
+      getInstance().billingAccountMap =
-        getInstance().billingAccountMap = null;
+          nullToEmptyImmutableCopy(billingAccountMap).entrySet().stream()
-      } else {
+              .collect(toImmutableMap(Map.Entry::getKey, BillingAccountEntry::new));
-        getInstance().billingAccountMap =
-            billingAccountMap.entrySet().stream()
-                .collect(toImmutableMap(Map.Entry::getKey, BillingAccountEntry::new));
-      }
       return this;
     }
 
@@ -1015,6 +1012,20 @@ public class Registrar extends ImmutableObject
           String.format(
               "Supplied IANA ID is not valid for %s registrar type: %s",
               getInstance().type, getInstance().ianaIdentifier));
+
+      // In order to grant access to real TLDs, the registrar must have a corresponding billing
+      // account ID for that TLD's billing currency.
+      ImmutableSet<String> nonBillableTlds =
+          Registry.get(getInstance().getAllowedTlds()).stream()
+              .filter(r -> r.getTldType() == TldType.REAL)
+              .filter(r -> !getInstance().getBillingAccountMap().containsKey(r.getCurrency()))
+              .map(Registry::getTldStr)
+              .collect(toImmutableSet());
+      checkArgument(
+          nonBillableTlds.isEmpty(),
+          "Cannot set these allowed, real TLDs because their currency is missing "
+              + "from the billing account map: %s",
+          nonBillableTlds);
       return cloneEmptyToNull(super.build());
     }
   }

core/src/main/java/google/registry/model/tld/Registries.java

@@ -72,7 +72,7 @@ public final class Registries {
                                 .stream()
                                 .map(Key::getName)
                                 .collect(toImmutableSet());
-                        return Registry.getAll(tlds).stream()
+                        return Registry.get(tlds).stream()
                             .map(e -> Maps.immutableEntry(e.getTldStr(), e.getTldType()))
                             .collect(entriesToImmutableMap());
                       } else {
@@ -105,7 +105,7 @@ public final class Registries {
 
   /** Returns the Registry entities themselves of the given type loaded fresh from Datastore. */
   public static ImmutableSet<Registry> getTldEntitiesOfType(TldType type) {
-    return Registry.getAll(filterValues(cache.get(), equalTo(type)).keySet());
+    return Registry.get(filterValues(cache.get(), equalTo(type)).keySet());
   }
 
   /** Pass-through check that the specified TLD exists, otherwise throw an IAE. */

core/src/main/java/google/registry/model/tld/Registry.java

@@ -142,10 +142,16 @@ public class Registry extends ImmutableObject
 
   /** The type of TLD, which determines things like backups and escrow policy. */
   public enum TldType {
-    /** A real, official TLD. */
+    /**
+     * A real, official TLD (but not necessarily only on production).
+     *
+     * <p>Note that, to avoid unnecessary costly DB writes, {@link
+     * google.registry.model.reporting.DomainTransactionRecord}s are only written out for REAL TLDs
+     * (these transaction records are only used for ICANN reporting purposes).
+     */
     REAL,
 
-    /** A test TLD, for the prober. */
+    /** A test TLD, for the prober, OT&amp;E, and other testing purposes. */
     TEST
   }
 
@@ -232,7 +238,7 @@ public class Registry extends ImmutableObject
   }
 
   /** Returns the registry entities for the given TLD strings, throwing if any don't exist. */
-  static ImmutableSet<Registry> getAll(Set<String> tlds) {
+  public static ImmutableSet<Registry> get(Set<String> tlds) {
     try {
       ImmutableMap<String, Optional<Registry>> registries = CACHE.getAll(tlds);
       ImmutableSet<String> missingRegistries =
@@ -1000,7 +1006,7 @@ public class Registry extends ImmutableObject
       instance.renewBillingCostTransitions.checkValidity();
       instance.eapFeeSchedule.checkValidity();
       // All costs must be in the expected currency.
-      // TODO(b/21854155): When we move PremiumList into Datastore, verify its currency too.
+      checkArgumentNotNull(instance.getCurrency(), "Currency must be set");
       checkArgument(
           instance.getStandardCreateCost().getCurrencyUnit().equals(instance.currency),
           "Create cost must be in the registry's currency");

core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java

@@ -24,14 +24,11 @@ import static java.nio.charset.StandardCharsets.US_ASCII;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameter;
-import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
 import google.registry.flows.certs.CertificateChecker;
 import google.registry.model.registrar.Registrar;
 import google.registry.model.registrar.RegistrarAddress;
-import google.registry.model.tld.Registry;
 import google.registry.tools.params.KeyValueMapParameter.CurrencyUnitToStringMap;
 import google.registry.tools.params.OptionalLongParameter;
 import google.registry.tools.params.OptionalPhoneNumberParameter;
@@ -46,7 +43,6 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 import org.joda.money.CurrencyUnit;
@@ -433,22 +429,6 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
         // Require a phone passcode.
         checkArgument(
             newRegistrar.getPhonePasscode() != null, "--passcode is required for REAL registrars.");
-        // Check if registrar has billing account IDs for the currency of the TLDs that it is
-        // allowed to register.
-        ImmutableSet<CurrencyUnit> tldCurrencies =
-            newRegistrar
-                .getAllowedTlds()
-                .stream()
-                .map(tld -> Registry.get(tld).getCurrency())
-                .collect(toImmutableSet());
-        Set<CurrencyUnit> currenciesWithoutBillingAccountId =
-            newRegistrar.getBillingAccountMap() == null
-                ? tldCurrencies
-                : Sets.difference(tldCurrencies, newRegistrar.getBillingAccountMap().keySet());
-        checkArgument(
-            currenciesWithoutBillingAccountId.isEmpty(),
-            "Need billing account map entries for currencies: %s",
-            Joiner.on(' ').join(currenciesWithoutBillingAccountId));
       }
 
       stageEntityChange(oldRegistrar, newRegistrar);

core/src/main/java/google/registry/tools/RegistryTool.java

@@ -15,6 +15,7 @@
 package google.registry.tools;
 
 import com.google.common.collect.ImmutableMap;
+import google.registry.tools.javascrap.BackfillRegistrarBillingAccountsCommand;
 import google.registry.tools.javascrap.CompareEscrowDepositsCommand;
 import google.registry.tools.javascrap.HardDeleteHostCommand;
 
@@ -30,6 +31,7 @@ public final class RegistryTool {
   public static final ImmutableMap<String, Class<? extends Command>> COMMAND_MAP =
       new ImmutableMap.Builder<String, Class<? extends Command>>()
           .put("ack_poll_messages", AckPollMessagesCommand.class)
+          .put("backfill_registrar_billing_accounts", BackfillRegistrarBillingAccountsCommand.class)
           .put("canonicalize_labels", CanonicalizeLabelsCommand.class)
           .put("check_domain", CheckDomainCommand.class)
           .put("check_domain_claims", CheckDomainClaimsCommand.class)

core/src/main/java/google/registry/tools/RegistryToolComponent.java

@@ -43,6 +43,7 @@ import google.registry.request.Modules.UrlConnectionServiceModule;
 import google.registry.request.Modules.UrlFetchServiceModule;
 import google.registry.request.Modules.UserServiceModule;
 import google.registry.tools.AuthModule.LocalCredentialModule;
+import google.registry.tools.javascrap.BackfillRegistrarBillingAccountsCommand;
 import google.registry.tools.javascrap.CompareEscrowDepositsCommand;
 import google.registry.tools.javascrap.HardDeleteHostCommand;
 import google.registry.util.UtilsModule;
@@ -90,6 +91,8 @@ import javax.inject.Singleton;
 interface RegistryToolComponent {
   void inject(AckPollMessagesCommand command);
 
+  void inject(BackfillRegistrarBillingAccountsCommand command);
+
   void inject(CheckDomainClaimsCommand command);
 
   void inject(CheckDomainCommand command);

core/src/main/java/google/registry/tools/javascrap/BackfillRegistrarBillingAccountsCommand.java

@@ -0,0 +1,56 @@
+// Copyright 2021 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.tools.javascrap;
+
+import static com.google.common.base.Preconditions.checkState;
+import static google.registry.model.OteAccountBuilder.DEFAULT_BILLING_ACCOUNT_MAP;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
+
+import com.beust.jcommander.Parameters;
+import google.registry.config.RegistryEnvironment;
+import google.registry.model.registrar.Registrar;
+import google.registry.tools.CommandWithRemoteApi;
+
+/**
+ * Backfills the billing account maps on all Registrars that don't have any set.
+ *
+ * <p>This should not (and cannot) be used on production. Its purpose is to backfill these values on
+ * sandbox, where most registrars have an empty billing account map, including all that were created
+ * for OT&amp;E. The same default billing account map is used for all registrars, and includes
+ * values for USD and JPY. The actual values here don't matter as we don't do invoicing on sandbox
+ * anyway.
+ */
+@Parameters(separators = " =", commandDescription = "Backfill registrar billing account maps.")
+public class BackfillRegistrarBillingAccountsCommand implements CommandWithRemoteApi {
+
+  @Override
+  public void run() throws Exception {
+    checkState(
+        RegistryEnvironment.get() != RegistryEnvironment.PRODUCTION,
+        "Do not run this on production");
+    System.out.println("Populating billing account maps on all registrars missing them ...");
+    tm().transact(
+            () ->
+                tm().loadAllOfStream(Registrar.class)
+                    .filter(r -> r.getBillingAccountMap().isEmpty())
+                    .forEach(
+                        r ->
+                            tm().update(
+                                    r.asBuilder()
+                                        .setBillingAccountMap(DEFAULT_BILLING_ACCOUNT_MAP)
+                                        .build())));
+    System.out.println("Done!");
+  }
+}

core/src/test/java/google/registry/export/sheet/SyncRegistrarsSheetTest.java

@@ -336,7 +336,11 @@ public class SyncRegistrarsSheetTest {
 
   @TestOfyAndSql
   void testRun_missingValues_stillWorks() throws Exception {
-    persistNewRegistrar("SomeRegistrar", "Some Registrar", Registrar.Type.REAL, 8L);
+    persistResource(
+        persistNewRegistrar("SomeRegistrar", "Some Registrar", Registrar.Type.REAL, 8L)
+            .asBuilder()
+            .setBillingAccountMap(ImmutableMap.of())
+            .build());
 
     newSyncRegistrarsSheet().run("foobar");
 

core/src/test/java/google/registry/model/registrar/RegistrarTest.java

@@ -30,11 +30,14 @@ import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.testing.DatabaseHelper.persistSimpleResource;
 import static google.registry.testing.DatabaseHelper.persistSimpleResources;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.joda.money.CurrencyUnit.JPY;
+import static org.joda.money.CurrencyUnit.USD;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.ImmutableSortedSet;
 import com.googlecode.objectify.Key;
 import google.registry.config.RegistryConfig;
@@ -42,13 +45,17 @@ import google.registry.model.EntityTestCase;
 import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.Registrar.Type;
 import google.registry.model.tld.Registries;
+import google.registry.model.tld.Registry;
+import google.registry.model.tld.Registry.TldType;
 import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.TestOfyAndSql;
 import google.registry.testing.TestOfyOnly;
 import google.registry.testing.TestSqlOnly;
 import google.registry.util.CidrAddressBlock;
 import google.registry.util.SerializeUtils;
+import java.math.BigDecimal;
 import org.joda.money.CurrencyUnit;
+import org.joda.money.Money;
 import org.junit.jupiter.api.BeforeEach;
 
 /** Unit tests for {@link Registrar}. */
@@ -60,7 +67,20 @@ class RegistrarTest extends EntityTestCase {
 
   @BeforeEach
   void setUp() {
-    createTld("xn--q9jyb4c");
+    createTld("tld");
+    persistResource(
+        newRegistry("xn--q9jyb4c", "MINNA")
+            .asBuilder()
+            .setCurrency(JPY)
+            .setCreateBillingCost(Money.of(JPY, new BigDecimal(1300)))
+            .setRestoreBillingCost(Money.of(JPY, new BigDecimal(1700)))
+            .setServerStatusChangeBillingCost(Money.of(JPY, new BigDecimal(1900)))
+            .setRegistryLockOrUnlockBillingCost(Money.of(JPY, new BigDecimal(2700)))
+            .setRenewBillingCostTransitions(
+                ImmutableSortedMap.of(START_OF_TIME, Money.of(JPY, new BigDecimal(1100))))
+            .setEapFeeSchedule(ImmutableSortedMap.of(START_OF_TIME, Money.zero(JPY)))
+            .setPremiumList(null)
+            .build());
     // Set up a new persisted registrar entity.
     registrar =
         cloneAndSetAutoTimestamps(
@@ -251,8 +271,10 @@ class RegistrarTest extends EntityTestCase {
   }
 
   @TestOfyAndSql
-  void testSuccess_clearingBillingAccountMap() {
+  void testSuccess_clearingBillingAccountMapAndAllowedTlds() {
-    registrar = registrar.asBuilder().setBillingAccountMap(null).build();
+    registrar =
+        registrar.asBuilder().setAllowedTlds(ImmutableSet.of()).setBillingAccountMap(null).build();
+    assertThat(registrar.getAllowedTlds()).isEmpty();
     assertThat(registrar.getBillingAccountMap()).isEmpty();
   }
 
@@ -677,4 +699,48 @@ class RegistrarTest extends EntityTestCase {
         assertThrows(IllegalArgumentException.class, () -> Registrar.loadByRegistrarIdCached(""));
     assertThat(thrown).hasMessageThat().contains("registrarId must be specified");
   }
+
+  @TestOfyAndSql
+  void testFailure_missingCurrenciesFromBillingMap() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                registrar
+                    .asBuilder()
+                    .setBillingAccountMap(null)
+                    .setAllowedTlds(ImmutableSet.of("tld", "xn--q9jyb4c"))
+                    .build());
+    assertThat(thrown)
+        .hasMessageThat()
+        .contains("their currency is missing from the billing account map: [tld, xn--q9jyb4c]");
+  }
+
+  @TestOfyAndSql
+  void testFailure_missingCurrencyFromBillingMap() {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                registrar
+                    .asBuilder()
+                    .setBillingAccountMap(ImmutableMap.of(USD, "abc123"))
+                    .setAllowedTlds(ImmutableSet.of("tld", "xn--q9jyb4c"))
+                    .build());
+    assertThat(thrown)
+        .hasMessageThat()
+        .contains("their currency is missing from the billing account map: [xn--q9jyb4c]");
+  }
+
+  @TestOfyAndSql
+  void testSuccess_nonRealTldDoesntNeedEntryInBillingMap() {
+    persistResource(Registry.get("xn--q9jyb4c").asBuilder().setTldType(TldType.TEST).build());
+    // xn--q9jyb4c bills in JPY and we don't have a JPY entry in this billing account map, but it
+    // should succeed without throwing an error because xn--q9jyb4c is set to be a TEST TLD.
+    registrar
+        .asBuilder()
+        .setBillingAccountMap(ImmutableMap.of(USD, "abc123"))
+        .setAllowedTlds(ImmutableSet.of("tld", "xn--q9jyb4c"))
+        .build();
+  }
 }

core/src/test/java/google/registry/model/tld/RegistryTest.java

@@ -190,7 +190,7 @@ public final class RegistryTest extends EntityTestCase {
   @TestOfyAndSql
   void testGetAll() {
     createTld("foo");
-    assertThat(Registry.getAll(ImmutableSet.of("foo", "tld")))
+    assertThat(Registry.get(ImmutableSet.of("foo", "tld")))
         .containsExactlyElementsIn(
             tm().transact(
                     () ->

core/src/test/java/google/registry/testing/AppEngineExtension.java

@@ -66,6 +66,7 @@ import java.util.Set;
 import java.util.logging.LogManager;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
+import org.joda.money.CurrencyUnit;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -289,6 +290,7 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
                 .build())
         .setPhoneNumber("+1.3334445555")
         .setPhonePasscode("12345")
+        .setBillingAccountMap(ImmutableMap.of(CurrencyUnit.USD, "abc123"))
         .setContactsRequireSyncing(true);
   }
 

core/src/test/java/google/registry/testing/DatabaseHelper.java

@@ -762,6 +762,7 @@ public class DatabaseHelper {
             .setType(type)
             .setState(State.ACTIVE)
             .setIanaIdentifier(ianaIdentifier)
+            .setBillingAccountMap(ImmutableMap.of(USD, "abc123"))
             .setLocalizedAddress(
                 new RegistrarAddress.Builder()
                     .setStreet(ImmutableList.of("123 Fake St"))

core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java

@@ -21,8 +21,11 @@ import static google.registry.testing.CertificateSamples.SAMPLE_CERT;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH;
 import static google.registry.testing.DatabaseHelper.createTlds;
+import static google.registry.testing.DatabaseHelper.newRegistry;
 import static google.registry.testing.DatabaseHelper.persistNewRegistrar;
+import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.joda.money.CurrencyUnit.JPY;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.verify;
@@ -43,8 +46,10 @@ import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.InjectExtension;
 import google.registry.testing.TestOfyAndSql;
 import java.io.IOException;
+import java.math.BigDecimal;
 import java.util.Optional;
 import org.joda.money.CurrencyUnit;
+import org.joda.money.Money;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -604,11 +609,11 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
     Optional<Registrar> registrar = Registrar.loadByRegistrarId("clientz");
     assertThat(registrar).isPresent();
     assertThat(registrar.get().getBillingAccountMap())
-        .containsExactly(CurrencyUnit.USD, "abc123", CurrencyUnit.JPY, "789xyz");
+        .containsExactly(CurrencyUnit.USD, "abc123", JPY, "789xyz");
   }
 
   @TestOfyAndSql
-  void testFailure_billingAccountMap_doesNotContainEntryForTldAllowed() {
+  void testFailure_billingAccountMap_doesNotContainEntryForAllowedTld() {
     createTlds("foo");
 
     IllegalArgumentException thrown =
@@ -632,12 +637,26 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
                     "--cc US",
                     "--force",
                     "clientz"));
-    assertThat(thrown).hasMessageThat().contains("USD");
+    assertThat(thrown)
+        .hasMessageThat()
+        .contains("their currency is missing from the billing account map: [foo]");
   }
 
   @TestOfyAndSql
   void testSuccess_billingAccountMap_onlyAppliesToRealRegistrar() throws Exception {
-    createTlds("foo");
+    persistResource(
+        newRegistry("foo", "FOO")
+            .asBuilder()
+            .setCurrency(JPY)
+            .setCreateBillingCost(Money.of(JPY, new BigDecimal(1300)))
+            .setRestoreBillingCost(Money.of(JPY, new BigDecimal(1700)))
+            .setServerStatusChangeBillingCost(Money.of(JPY, new BigDecimal(1900)))
+            .setRegistryLockOrUnlockBillingCost(Money.of(JPY, new BigDecimal(2700)))
+            .setRenewBillingCostTransitions(
+                ImmutableSortedMap.of(START_OF_TIME, Money.of(JPY, new BigDecimal(1100))))
+            .setEapFeeSchedule(ImmutableSortedMap.of(START_OF_TIME, Money.zero(JPY)))
+            .setPremiumList(null)
+            .build());
 
     runCommandForced(
         "--name=blobio",
@@ -656,7 +675,7 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
 
     Optional<Registrar> registrar = Registrar.loadByRegistrarId("clientz");
     assertThat(registrar).isPresent();
-    assertThat(registrar.get().getBillingAccountMap()).containsExactly(CurrencyUnit.JPY, "789xyz");
+    assertThat(registrar.get().getBillingAccountMap()).containsExactly(JPY, "789xyz");
   }
 
   @TestOfyAndSql

core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java

@@ -21,8 +21,10 @@ import static google.registry.testing.CertificateSamples.SAMPLE_CERT3;
 import static google.registry.testing.CertificateSamples.SAMPLE_CERT3_HASH;
 import static google.registry.testing.DatabaseHelper.createTlds;
 import static google.registry.testing.DatabaseHelper.loadRegistrar;
+import static google.registry.testing.DatabaseHelper.newRegistry;
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.joda.money.CurrencyUnit.JPY;
 import static org.joda.time.DateTimeZone.UTC;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -38,8 +40,10 @@ import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.Registrar.Type;
 import google.registry.testing.AppEngineExtension;
 import google.registry.util.CidrAddressBlock;
+import java.math.BigDecimal;
 import java.util.Optional;
 import org.joda.money.CurrencyUnit;
+import org.joda.money.Money;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -359,6 +363,8 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
 
   @Test
   void testSuccess_billingAccountMap() throws Exception {
+    persistResource(
+        loadRegistrar("NewRegistrar").asBuilder().setBillingAccountMap(ImmutableMap.of()).build());
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
     runCommand("--billing_account_map=USD=abc123,JPY=789xyz", "--force", "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())
@@ -366,8 +372,14 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
   }
 
   @Test
-  void testFailure_billingAccountMap_doesNotContainEntryForTldAllowed() {
+  void testFailure_billingAccountMap_doesNotContainEntryForAllowedTld() {
     createTlds("foo");
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
+            .setAllowedTlds(ImmutableSet.of())
+            .setBillingAccountMap(ImmutableMap.of())
+            .build());
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
     IllegalArgumentException thrown =
         assertThrows(
@@ -379,12 +391,28 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
                     "--force",
                     "--registrar_type=REAL",
                     "NewRegistrar"));
-    assertThat(thrown).hasMessageThat().contains("USD");
+    assertThat(thrown)
+        .hasMessageThat()
+        .contains("their currency is missing from the billing account map: [foo]");
   }
 
   @Test
   void testSuccess_billingAccountMap_onlyAppliesToRealRegistrar() throws Exception {
-    createTlds("foo");
+    persistResource(
+        newRegistry("foo", "FOO")
+            .asBuilder()
+            .setCurrency(JPY)
+            .setCreateBillingCost(Money.of(JPY, new BigDecimal(1300)))
+            .setRestoreBillingCost(Money.of(JPY, new BigDecimal(1700)))
+            .setServerStatusChangeBillingCost(Money.of(JPY, new BigDecimal(1900)))
+            .setRegistryLockOrUnlockBillingCost(Money.of(JPY, new BigDecimal(2700)))
+            .setRenewBillingCostTransitions(
+                ImmutableSortedMap.of(START_OF_TIME, Money.of(JPY, new BigDecimal(1100))))
+            .setEapFeeSchedule(ImmutableSortedMap.of(START_OF_TIME, Money.zero(JPY)))
+            .setPremiumList(null)
+            .build());
+    persistResource(
+        loadRegistrar("NewRegistrar").asBuilder().setBillingAccountMap(ImmutableMap.of()).build());
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
     runCommand("--billing_account_map=JPY=789xyz", "--allowed_tlds=foo", "--force", "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())