Skip to content
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.

Sign up for GitHub

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

fix: multiple deposits of unknown brand #7027

Merged
merged 3 commits into from
Feb 17, 2023
Merged

fix: multiple deposits of unknown brand #7027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ef6062
fix: multiple deposits of unknown brand
turadg Feb 17, 2023
45bff83
test(smoketest): realistic ATOM price
turadg Feb 16, 2023
fefc85e
Merge branch 'master' into 6961-save-unknown-payments
mergify[bot] Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/agoric-cli/test/agops-oracle-smoketest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,14 @@ agd query vstorage keys published.priceFeed
# verify that the round started
agoric follow :published.priceFeed.ATOM-USD_price_feed.latestRound

# Set it to $13 per ATOM
# second round, first oracle
PROPOSAL_OFFER=$(mktemp -t agops.XXX)
bin/agops oracle pushPriceRound --price 1102 --roundId 2 --oracleAdminAcceptOfferId "$ORACLE_OFFER_ID" >|"$PROPOSAL_OFFER"
bin/agops oracle pushPriceRound --price 120000000 --roundId 2 --oracleAdminAcceptOfferId "$ORACLE_OFFER_ID" >|"$PROPOSAL_OFFER"
agoric wallet send --offer "$PROPOSAL_OFFER" --from gov1 --keyring-backend="test"
# second round, second oracle
PROPOSAL_OFFER=$(mktemp -t agops.XXX)
bin/agops oracle pushPriceRound --price 1202 --roundId 2 --oracleAdminAcceptOfferId "$ORACLE2_OFFER_ID" >|"$PROPOSAL_OFFER"
bin/agops oracle pushPriceRound --price 14000000 --roundId 2 --oracleAdminAcceptOfferId "$ORACLE2_OFFER_ID" >|"$PROPOSAL_OFFER"
agoric wallet send --offer "$PROPOSAL_OFFER" --from gov2 --keyring-backend="test"

# see new price
Expand Down
21 changes: 5 additions & 16 deletions packages/inter-protocol/test/smartWallet/test-psm-integration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,26 +307,15 @@ test('govern offerFilter', async t => {
// vote didn't raise an error.
});

test('deposit unknown brand', async t => {
const rial = withAmountUtils(makeIssuerKit('rial'));
assert(rial.mint);

const wallet = await t.context.simpleProvideWallet('agoric1queue');

const payment = rial.mint.mintPayment(rial.make(1_000n));
// @ts-expect-error deposit does take a FarRef<Payment>
const result = await wallet.getDepositFacet().receive(harden(payment));
// successful request but not deposited
t.deepEqual(result, { brand: rial.brand, value: 0n });
});

test.failing('deposit > 1 payment to unknown brand #6961', async t => {
// XXX belongs in smart-wallet package, but needs lots of set-up that's handy here.
test('deposit multiple payments to unknown brand', async t => {
const rial = withAmountUtils(makeIssuerKit('rial'));

const wallet = await t.context.simpleProvideWallet('agoric1queue');

for await (const _ of [1, 2]) {
const payment = rial.mint.mintPayment(rial.make(1_000n));
// assume that if the call succeeds then it's in durable storage.
for await (const amt of [1n, 2n]) {
const payment = rial.mint.mintPayment(rial.make(amt));
// @ts-expect-error deposit does take a FarRef<Payment>
const result = await wallet.getDepositFacet().receive(harden(payment));
// successful request but not deposited
Expand Down
10 changes: 6 additions & 4 deletions packages/smart-wallet/src/smartWallet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ export const prepareSmartWallet = (baggage, shared) => {
/**
* Put the assets from the payment into the appropriate purse.
*
* If the purse doesn't exist, we hold the payment until it does.
* If the purse doesn't exist, we hold the payment in durable storage.
*
* @param {import('@endo/far').FarRef<Payment>} payment
* @returns {Promise<Amount>} amounts for deferred deposits will be empty
Expand All @@ -374,11 +374,13 @@ export const prepareSmartWallet = (baggage, shared) => {
return E(invitationPurse).deposit(payment);
}

// When there is no purse, queue the payment
// When there is no purse, save the payment into a queue.
// It's not yet ever read but a future version of the contract can
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting... so we don't yet trigger deposits when new issuers come along, but we support upgrade to a wallet that does.

Hm... is there a way for somebody to send an ERTP payment with a brand not known to the vbank? I can't think of one. I was thinking we might owe an issue for the case of newly-approved tokens, but I think we're internally consistent as-is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's right

if (queues.has(brand)) {
queues.get(brand).push(payment);
const extant = queues.get(brand);
queues.set(brand, harden([...extant, payment]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to be the gist of it.

} else {
queues.init(brand, harden([payment])); // TODO: #6961 fix this.
queues.init(brand, harden([payment]));
}
return AmountMath.makeEmpty(brand);
},
Expand Down