Skip to content

Commit

Permalink
chore: advancer validates settlementAccount address (#10723)
Browse files Browse the repository at this point in the history
closes: #10726

## Description
Primary aim is to validate evidence reported to FUSDC contains the expected `settlementAccount` address. This supports **C8 - Contract MUST be able to initialize settlement process when Noble mints USDC**.

To facilitate this change:
- add `makeTestAddress` helper in `@agoric/orchestration/tools` that returns a valid bech32 address
   - since `encodeAddressHook` throws when presented `agoric1FakeLCAAddress`
- bootstrap's `VLOCALCHAIN_ALLOCATE_ADDRESS`  now returns a valid (mocked) bech32 address
- `makeFakeLocalchainBridge` accepts `makeAddressFn`

### Security Considerations
Hardens FUSDC implementation.

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
Includes new tests (see advancer.test.ts) and updates existing tests

### Upgrade Considerations
None, unreleased code
  • Loading branch information
mergify[bot] authored Dec 18, 2024
2 parents c1d2b71 + e06f12a commit 7b7ceb9
Show file tree
Hide file tree
Showing 18 changed files with 191 additions and 63 deletions.
14 changes: 11 additions & 3 deletions packages/boot/test/bootstrapTests/orchestration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { SIMULATED_ERRORS } from '@agoric/vats/tools/fake-bridge.js';
import fetchedChainInfo from '@agoric/orchestration/src/fetched-chain-info.js';
import { buildVTransferEvent } from '@agoric/orchestration/tools/ibc-mocks.js';
import { BridgeId } from '@agoric/internal';
import { makeTestAddress } from '@agoric/orchestration/tools/make-test-address.js';
import {
makeWalletFactoryContext,
type WalletFactoryTestContext,
Expand Down Expand Up @@ -410,8 +411,9 @@ test.serial('basic-flows', async t => {
const publicSubscriberPaths = Object.fromEntries(
wd.getCurrentWalletRecord().offerToPublicSubscriberPaths,
);
const expectedAddress = makeTestAddress();
t.deepEqual(publicSubscriberPaths['request-loa'], {
account: 'published.basicFlows.agoric1fakeLCAAddress',
account: `published.basicFlows.${expectedAddress}`,
});
t.like(wd.getLatestUpdateRecord(), {
status: { id: 'request-loa', numWantsSatisfied: 1 },
Expand Down Expand Up @@ -487,6 +489,8 @@ test.serial('basic-flows', async t => {
await runInbound(
BridgeId.VTRANSFER,
buildVTransferEvent({
sender: expectedAddress,
target: expectedAddress,
sourceChannel: 'channel-62',
sequence: '1',
}),
Expand Down Expand Up @@ -598,7 +602,8 @@ test.serial('basic-flows - portfolio holder', async t => {
[
'request-portfolio-acct',
{
agoric: 'published.basicFlows.agoric1fakeLCAAddress1',
agoric:
'published.basicFlows.agoric1qyqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqc09z0g',
cosmoshub: 'published.basicFlows.cosmos1test2',
osmosis: 'published.basicFlows.cosmos1test3',
},
Expand All @@ -615,7 +620,10 @@ test.serial('basic-flows - portfolio holder', async t => {
remoteAddress:
'/ibc-hop/connection-1/ibc-port/icahost/ordered/{"version":"ics27-1","controllerConnectionId":"connection-1","hostConnectionId":"connection-1649","address":"cosmos1test3","encoding":"proto3","txType":"sdk_multi_msg"}/ibc-channel/channel-4',
});
t.is(readPublished('basicFlows.agoric1fakeLCAAddress1'), '');
t.is(
readPublished('basicFlows.agoric1qyqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqc09z0g'),
'',
);

const { BLD } = agoricNamesRemotes.brand;
BLD || Fail`BLD missing from agoricNames`;
Expand Down
11 changes: 9 additions & 2 deletions packages/boot/test/fast-usdc/fast-usdc.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js';

import type { TestFn } from 'ava';
import { encodeAddressHook } from '@agoric/cosmic-proto/address-hooks.js';
import { configurations } from '@agoric/fast-usdc/src/utils/deploy-config.js';
import { MockCctpTxEvidences } from '@agoric/fast-usdc/test/fixtures.js';
import { documentStorageSchema } from '@agoric/governance/tools/storageDoc.js';
Expand All @@ -12,7 +13,7 @@ import {
defaultSerializer,
} from '@agoric/internal/src/storage-test-utils.js';
import { eventLoopIteration } from '@agoric/internal/src/testing-utils.js';
import { BridgeId } from '@agoric/internal';
import { BridgeId, NonNullish } from '@agoric/internal';
import {
makeWalletFactoryContext,
type WalletFactoryTestContext,
Expand Down Expand Up @@ -261,7 +262,13 @@ test.serial('makes usdc advance', async t => {
'FastLP balance not in wallet record',
);

const evidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO();
const EUD = 'dydx1anything';
const lastNodeValue = storage.getValues('published.fastUsdc').at(-1);
const { settlementAccount } = JSON.parse(NonNullish(lastNodeValue));
const evidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(
// mock with the read settlementAccount address
encodeAddressHook(settlementAccount, { EUD }),
);

harness?.useRunPolicy(true);
await Promise.all(
Expand Down
8 changes: 4 additions & 4 deletions packages/boot/test/fast-usdc/snapshots/fast-usdc.test.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,19 @@ Generated by [AVA](https://avajs.dev).
[
'published.fastUsdc',
{
poolAccount: 'agoric1fakeLCAAddress',
settlementAccount: 'agoric1fakeLCAAddress1',
poolAccount: 'agoric1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqp7zqht',
settlementAccount: 'agoric1qyqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqc09z0g',
},
],
[
'published.fastUsdc.agoric1fakeLCAAddress',
'published.fastUsdc.agoric1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqp7zqht',
{
body: '#""',
slots: [],
},
],
[
'published.fastUsdc.agoric1fakeLCAAddress1',
'published.fastUsdc.agoric1qyqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqc09z0g',
{
body: '#""',
slots: [],
Expand Down
Binary file modified packages/boot/test/fast-usdc/snapshots/fast-usdc.test.ts.snap
Binary file not shown.
3 changes: 3 additions & 0 deletions packages/boot/test/orchestration/contract-upgrade.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { BridgeId } from '@agoric/internal';
import { buildVTransferEvent } from '@agoric/orchestration/tools/ibc-mocks.js';
import fetchedChainInfo from '@agoric/orchestration/src/fetched-chain-info.js';
import { withChainCapabilities } from '@agoric/orchestration';
import { makeTestAddress } from '@agoric/orchestration/tools/make-test-address.js';
import {
makeWalletFactoryContext,
type WalletFactoryTestContext,
Expand Down Expand Up @@ -100,6 +101,8 @@ test('resume', async t => {
await runInbound(
BridgeId.VTRANSFER,
buildVTransferEvent({
sender: makeTestAddress(),
target: makeTestAddress(),
sourceChannel: 'channel-5',
sequence: '1',
}),
Expand Down
3 changes: 3 additions & 0 deletions packages/boot/test/orchestration/restart-contracts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import { buildVTransferEvent } from '@agoric/orchestration/tools/ibc-mocks.js';
import type { UpdateRecord } from '@agoric/smart-wallet/src/smartWallet.js';
import fetchedChainInfo from '@agoric/orchestration/src/fetched-chain-info.js';
import { makeTestAddress } from '@agoric/orchestration/tools/make-test-address.js';
import {
makeWalletFactoryContext,
type WalletFactoryTestContext,
Expand Down Expand Up @@ -101,6 +102,8 @@ test.serial('send-anywhere', async t => {
await runInbound(
BridgeId.VTRANSFER,
buildVTransferEvent({
sender: makeTestAddress(),
target: makeTestAddress(),
sourceChannel: 'channel-5',
sequence: '1',
}),
Expand Down
3 changes: 2 additions & 1 deletion packages/boot/tools/supports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { resolve as importMetaResolve } from 'import-meta-resolve';
import { basename, join } from 'path';
import { inspect } from 'util';

import { makeTestAddress } from '@agoric/orchestration/tools/make-test-address.js';
import { buildSwingset } from '@agoric/cosmic-swingset/src/launch-chain.js';
import type { TypedPublished } from '@agoric/client-utils';
import {
Expand Down Expand Up @@ -538,7 +539,7 @@ export const makeSwingsetTestKit = async (
return undefined;
}
case `${BridgeId.VLOCALCHAIN}:VLOCALCHAIN_ALLOCATE_ADDRESS`: {
const address = `${LOCALCHAIN_DEFAULT_ADDRESS}${lcaAccountsCreated || ''}`;
const address = makeTestAddress(lcaAccountsCreated);
lcaAccountsCreated += 1;
return address;
}
Expand Down
11 changes: 9 additions & 2 deletions packages/fast-usdc/src/exos/advancer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { pickFacet } from '@agoric/vat-data';
import { VowShape } from '@agoric/vow';
import { E } from '@endo/far';
import { M, mustMatch } from '@endo/patterns';
import { Fail, q } from '@endo/errors';
import {
CctpTxEvidenceShape,
AddressHookShape,
Expand Down Expand Up @@ -116,6 +117,7 @@ export const prepareAdvancerKit = (
* notifyFacet: import('./settler.js').SettlerKit['notify'];
* borrowerFacet: LiquidityPoolKit['borrower'];
* poolAccount: HostInterface<OrchestrationAccount<{chainId: 'agoric'}>>;
* settlementAddress: ChainAddress;
* intermediateRecipient?: ChainAddress;
* }} config
*/
Expand Down Expand Up @@ -145,10 +147,14 @@ export const prepareAdvancerKit = (
return;
}

const { borrowerFacet, poolAccount } = this.state;
const { borrowerFacet, poolAccount, settlementAddress } =
this.state;
const { recipientAddress } = evidence.aux;
const decoded = decodeAddressHook(recipientAddress);
mustMatch(decoded, AddressHookShape);
if (decoded.baseAddress !== settlementAddress.value) {
throw Fail`⚠️ baseAddress of address hook ${q(decoded.baseAddress)} does not match the expected address ${q(settlementAddress.value)}`;
}
const { EUD } = /** @type {AddressHook['query']} */ (decoded.query);
log(`decoded EUD: ${EUD}`);
// throws if the bech32 prefix is not found
Expand All @@ -172,10 +178,10 @@ export const prepareAdvancerKit = (
harden({ USDC: advanceAmount }),
);
void watch(depositV, this.facets.depositHandler, {
fullAmount,
advanceAmount,
destination,
forwardingAddress: evidence.tx.forwardingAddress,
fullAmount,
tmpSeat,
txHash: evidence.txHash,
});
Expand Down Expand Up @@ -271,6 +277,7 @@ export const prepareAdvancerKit = (
borrowerFacet: M.remotable(),
poolAccount: M.remotable(),
intermediateRecipient: M.opt(ChainAddressShape),
settlementAddress: M.opt(ChainAddressShape),
}),
},
);
Expand Down
15 changes: 7 additions & 8 deletions packages/fast-usdc/src/fast-usdc.contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,12 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
},
async publishAddresses() {
!baggage.has(ADDRESSES_BAGGAGE_KEY) || Fail`Addresses already published`;
const [poolAccountAddress, settlementAccountAddress] =
await vowTools.when(
vowTools.all([
E(poolAccount).getAddress(),
E(settlementAccount).getAddress(),
]),
);
const [poolAccountAddress] = await vowTools.when(
vowTools.all([E(poolAccount).getAddress()]),
);
const addresses = harden({
poolAccount: poolAccountAddress.value,
settlementAccount: settlementAccountAddress.value,
settlementAccount: settlementAddress.value,
});
baggage.init(ADDRESSES_BAGGAGE_KEY, addresses);
await publishAddresses(storageNode, addresses);
Expand Down Expand Up @@ -284,6 +280,8 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
);
trace('settlementAccount', settlementAccount);
trace('poolAccount', poolAccount);
const settlementAddress = await E(settlementAccount).getAddress();
trace('settlementAddress', settlementAddress);

const [_agoric, _noble, agToNoble] = await vowTools.when(
chainHub.getChainsAndConnection('agoric', 'noble'),
Expand All @@ -300,6 +298,7 @@ export const contract = async (zcf, privateArgs, zone, tools) => {
borrowerFacet: poolKit.borrower,
notifyFacet: settlerKit.notify,
poolAccount,
settlementAddress,
}),
);
// Connect evidence stream to advancer
Expand Down
7 changes: 3 additions & 4 deletions packages/fast-usdc/test/cli/transfer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
makeFetchMock,
makeMockSigner,
} from '../../testing/mocks.js';
import { settlementAddress } from '../fixtures.js';

test('Errors if config missing', async t => {
const path = 'config/dir/.fast-usdc/config.json';
Expand Down Expand Up @@ -73,8 +74,7 @@ test('Transfer registers the noble forwarding account if it does not exist', asy
};
const out = mockOut();
const file = mockFile(path, JSON.stringify(config));
const agoricSettlementAccount =
'agoric16kv2g7snfc4q24vg3pjdlnnqgngtjpwtetd2h689nz09lcklvh5s8u37ek';
const agoricSettlementAccount = settlementAddress.value;
const settlementAccountVstoragePath = 'published.fastUsdc.settlementAccount';
const vstorageMock = makeVstorageMock({
[settlementAccountVstoragePath]: agoricSettlementAccount,
Expand Down Expand Up @@ -150,8 +150,7 @@ test('Transfer signs and broadcasts the depositForBurn message on Ethereum', asy
};
const out = mockOut();
const file = mockFile(path, JSON.stringify(config));
const agoricSettlementAccount =
'agoric16kv2g7snfc4q24vg3pjdlnnqgngtjpwtetd2h689nz09lcklvh5s8u37ek';
const agoricSettlementAccount = settlementAddress.value;
const settlementAccountVstoragePath = 'published.fastUsdc.settlementAccount';
const vstorageMock = makeVstorageMock({
[settlementAccountVstoragePath]: agoricSettlementAccount,
Expand Down
47 changes: 42 additions & 5 deletions packages/fast-usdc/test/exos/advancer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ import type { SettlerKit } from '../../src/exos/settler.js';
import { prepareStatusManager } from '../../src/exos/status-manager.js';
import type { LiquidityPoolKit } from '../../src/types.js';
import { makeFeeTools } from '../../src/utils/fees.js';
import { MockCctpTxEvidences, intermediateRecipient } from '../fixtures.js';
import {
MockCctpTxEvidences,
settlementAddress,
intermediateRecipient,
} from '../fixtures.js';
import {
makeTestFeeConfig,
makeTestLogger,
Expand Down Expand Up @@ -127,6 +131,7 @@ const createTestExtensions = (t, common: CommonSetup) => {
notifyFacet: mockNotifyF,
poolAccount: mockAccounts.mockPoolAccount.account,
intermediateRecipient,
settlementAddress,
});

return {
Expand All @@ -141,6 +146,7 @@ const createTestExtensions = (t, common: CommonSetup) => {
},
mocks: {
...mockAccounts,
mockBorrowerF,
mockNotifyF,
resolveLocalTransferV,
rejectLocalTransfeferV,
Expand Down Expand Up @@ -260,6 +266,7 @@ test('updates status to OBSERVED on insufficient pool funds', async t => {
notifyFacet: mockNotifyF,
poolAccount: mockPoolAccount.account,
intermediateRecipient,
settlementAddress,
});

const evidence = MockCctpTxEvidences.AGORIC_PLUS_DYDX();
Expand Down Expand Up @@ -393,10 +400,10 @@ test('updates status to OBSERVED if pre-condition checks fail', async t => {

await advancer.handleTransactionEvent({
...MockCctpTxEvidences.AGORIC_NO_PARAMS(
encodeAddressHook(
'agoric16kv2g7snfc4q24vg3pjdlnnqgngtjpwtetd2h689nz09lcklvh5s8u37ek',
{ EUD: 'osmo1234', extra: 'value' },
),
encodeAddressHook(settlementAddress.value, {
EUD: 'osmo1234',
extra: 'value',
}),
),
txHash:
'0xc81bc6105b60a234c7c50ac17816ebcd5561d366df8bf3be59ff387552761799',
Expand Down Expand Up @@ -550,6 +557,7 @@ test('alerts if `returnToPool` fallback fails', async t => {
notifyFacet: mockNotifyF,
poolAccount: mockPoolAccount.account,
intermediateRecipient,
settlementAddress,
});

const mockEvidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO();
Expand Down Expand Up @@ -588,3 +596,32 @@ test('alerts if `returnToPool` fallback fails', async t => {
'Advancing tx is recorded as AdvanceFailed',
);
});

test('rejects advances to unknown settlementAccount', async t => {
const {
extensions: {
services: { advancer },
helpers: { inspectLogs },
},
} = t.context;

const invalidSettlementAcct =
'agoric1ax7hmw49tmqrdld7emc5xw3wf43a49rtkacr9d5nfpqa0y7k6n0sl8v94h';
t.not(settlementAddress.value, invalidSettlementAcct);
const mockEvidence = MockCctpTxEvidences.AGORIC_PLUS_OSMO(
encodeAddressHook(invalidSettlementAcct, {
EUD: 'osmo183dejcnmkka5dzcu9xw6mywq0p2m5peks28men',
}),
);

void advancer.handleTransactionEvent(mockEvidence);
await eventLoopIteration();
t.deepEqual(inspectLogs(), [
[
'Advancer error:',
Error(
'⚠️ baseAddress of address hook "agoric1ax7hmw49tmqrdld7emc5xw3wf43a49rtkacr9d5nfpqa0y7k6n0sl8v94h" does not match the expected address "agoric16kv2g7snfc4q24vg3pjdlnnqgngtjpwtetd2h689nz09lcklvh5s8u37ek"',
),
],
]);
});
Loading

0 comments on commit 7b7ceb9

Please sign in to comment.