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.

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

contract audit fixes #145

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"image": "mcr.microsoft.com/devcontainers/universal:2",
"features": {
"ghcr.io/devcontainers/features/node:1": {}
}
}
21 changes: 10 additions & 11 deletions contract/Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
CHAINID=agoriclocal
USER1ADDR=$(kagd keys show user1 -a --keyring-backend="test")
USER1ADDR=$(agd keys show tg -a --keyring-backend="test")
ACCT_ADDR=$(USER1ADDR)
BLD=000000ubld

Expand All @@ -13,8 +13,8 @@ list:
awk -v RS= -F: '$$1 ~ /^[^#%]+$$/ { print $$1 }'

balance-q:
kagd keys show user1 -a --keyring-backend="test"
kagd query bank balances $(ACCT_ADDR)
agd keys show tg -a --keyring-backend="test"
agd query bank balances $(ACCT_ADDR)

GAS_ADJUSTMENT=1.2
SIGN_BROADCAST_OPTS=--keyring-backend=test --chain-id=$(CHAINID) \
Expand All @@ -25,40 +25,40 @@ mint100:
make FUNDS=1000$(ATOM) fund-acct
cd /usr/src/agoric-sdk && \
yarn --silent agops vaults open --wantMinted 100 --giveCollateral 100 >/tmp/want-ist.json && \
yarn --silent agops perf satisfaction --executeOffer /tmp/want-ist.json --from user1 --keyring-backend=test
yarn --silent agops perf satisfaction --executeOffer /tmp/want-ist.json --from tg --keyring-backend=test

# Keep mint4k around a while for compatibility
mint4k:
make FUNDS=1000$(ATOM) fund-acct
cd /usr/src/agoric-sdk && \
yarn --silent agops vaults open --wantMinted 4000 --giveCollateral 1000 >/tmp/want4k.json && \
yarn --silent agops perf satisfaction --executeOffer /tmp/want4k.json --from user1 --keyring-backend=test
yarn --silent agops perf satisfaction --executeOffer /tmp/want4k.json --from tg --keyring-backend=test

FUNDS=321$(BLD)
fund-acct:
kagd tx bank send validator $(ACCT_ADDR) $(FUNDS) \
agd tx bank send validator $(ACCT_ADDR) $(FUNDS) \
$(SIGN_BROADCAST_OPTS) \
-o json >,tx.json
jq '{code: .code, height: .height}' ,tx.json

gov-q:
kagd query gov proposals --output json | \
agd query gov proposals --output json | \
jq -c '.proposals[] | [.proposal_id,.voting_end_time,.status]'

gov-voting-q:
kagd query gov proposals --status=voting_period --output json | \
agd query gov proposals --status=voting_period --output json | \
jq -c '.proposals[].proposal_id'

PROPOSAL=1
VOTE_OPTION=yes
vote:
kagd tx gov vote $(PROPOSAL) $(VOTE_OPTION) --from=validator \
agd tx gov vote $(PROPOSAL) $(VOTE_OPTION) --from=validator \
$(SIGN_BROADCAST_OPTS) \
-o json >,tx.json
jq '{code: .code, height: .height}' ,tx.json

instance-q:
kagd query vstorage data published.agoricNames.instance -o json
agd query vstorage data published.agoricNames.instance -o json

start-contract: check-contract-airdrop

Expand All @@ -73,7 +73,6 @@ start: make start-contract-airdrop start-contract
start-contract-airdrop:
yarn node scripts/deploy-contract.js \
--install src/airdrop.contract.js \
--eval src/platform-goals/board-aux.core.js \
--eval src/airdrop.proposal.js

start-contract-swap:
Expand Down
2 changes: 1 addition & 1 deletion contract/scripts/deploy-contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const options = {
help: { type: 'boolean' },
install: { type: 'string' },
eval: { type: 'string', multiple: true },
service: { type: 'string', default: 'kagd' },
service: { type: 'string', default: 'agd' },
workdir: { type: 'string', default: '/workspace/contract' },
};
/**
Expand Down
106 changes: 90 additions & 16 deletions contract/src/airdrop.contract.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ import { E } from '@endo/far';
import { AmountMath, AmountShape, AssetKind, MintShape } from '@agoric/ertp';
import { TimeMath } from '@agoric/time';
import { TimerShape } from '@agoric/zoe/src/typeGuards.js';
import { bech32 } from 'bech32';
import { sha256 } from '@noble/hashes/sha256';
import { ripemd160 } from '@noble/hashes/ripemd160';
import {
atomicRearrange,
makeRatio,
withdrawFromSeat,
} from '@agoric/zoe/src/contractSupport/index.js';
import { decodeBase64 } from '@endo/base64';
import { divideBy } from '@agoric/zoe/src/contractSupport/ratio.js';
import { makeTracer } from '@agoric/internal';
import { makeTracer, mustMatch } from '@agoric/internal';
import { makeWaker, oneDay } from './helpers/time.js';
import {
handleFirstIncarnation,
Expand All @@ -23,6 +27,17 @@ import { objectToMap } from './helpers/objectTools.js';
import { getMerkleRootFromMerkleProof } from './merkle-tree/index.js';
import '@agoric/zoe/exported.js';

const ProofDataShape = harden({
hash: M.string(),
direction: M.string(),
});

const OfferArgsShape = harden({
tier: M.number(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

tier seems to be defined as bigInt in the contract and not as a number - not sure if that matters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ATPIT the tier argument is working correctly within the contract. That said, the tier value is going to be concatenated onto the end of the pubkey hex string rather than being its own argument.

key: M.string(),
proof: M.arrayOf(ProofDataShape),
});

const TT = makeTracer('ContractStartFn');

export const messagesObject = {
Expand Down Expand Up @@ -70,14 +85,13 @@ harden(RESTARTING);
/** @import {ContractMeta} from './@types/zoe-contract-facet.d'; */
/** @import {Remotable} from '@endo/marshal' */

export const privateArgsShape = harden({
marshaller: M.remotable('marshaller'),
storageNode: M.remotable('chainStorageNode'),
export const privateArgsShape = {
namesByAddress: M.remotable('marshaller'),
timer: TimerShape,
});
};
harden(privateArgsShape);

export const customTermsShape = harden({
export const customTermsShape = {
targetEpochLength: M.bigint(),
initialPayoutValues: M.arrayOf(M.bigint()),
tokenName: M.string(),
Expand All @@ -86,7 +100,7 @@ export const customTermsShape = harden({
startTime: M.bigint(),
feeAmount: AmountShape,
merkleRoot: M.string(),
});
};
harden(customTermsShape);

export const divideAmountByTwo = brand => amount =>
Expand Down Expand Up @@ -162,7 +176,27 @@ export const start = async (zcf, privateArgs, baggage) => {
/** @type {Zone} */
const zone = makeDurableZone(baggage, 'rootZone');

const { timer } = privateArgs;
const { timer, namesByAddress } = privateArgs;

/**
* @param {string} addr
* @returns {ERef<DepositFacet>}
*/
const getDepositFacet = addr => {
assert.typeof(addr, 'string');
console.log('geting deposit facet for::', addr);
const df = E(namesByAddress).lookup(addr, 'depositFacet');
console.log('------------------------');
console.log('df::', df);
return df;
};

/**
* @param {string} addr
* @param {Payment} pmt
*/
const sendTo = (addr, pmt) => E(getDepositFacet(addr)).receive(pmt);

/** @type {ContractTerms} */
const {
startTime = 120n,
Expand Down Expand Up @@ -345,18 +379,25 @@ export const start = async (zcf, privateArgs, baggage) => {
* }} offerArgs
*/
const claimHandler = (claimSeat, offerArgs) => {
mustMatch(
offerArgs,
OfferArgsShape,
'offerArgs does not contain the correct data.',
);
const {
give: { Fee: claimTokensFee },
} = claimSeat.getProposal();

const { proof, key: pubkey, address, tier } = offerArgs;
const { proof, key: pubkey, tier } = offerArgs;

const derivedAddress = computeAddress(pubkey);

// This line was added because of issues when testing
// Is there a way to gracefully test assertion failures????
if (accountStore.has(pubkey)) {
claimSeat.exit();
throw new Error(
`Allocation for address ${address} has already been claimed.`,
`Allocation for address ${derivedAddress} has already been claimed.`,
);
}

Expand All @@ -367,21 +408,54 @@ export const start = async (zcf, privateArgs, baggage) => {
);

const paymentAmount = this.state.payoutArray[tier];
console.log('------------------------');
console.log('paymentAmount::', paymentAmount);
const payment = await withdrawFromSeat(zcf, tokenHolderSeat, {
Tokens: paymentAmount,
});

console.log('------------------------');
console.log(
'withdrawn from tokenHolderSeat ### payment::',
payment,
);
console.log('------------------------');

const depositFacet = await getDepositFacet(derivedAddress);

console.log('------------------------');
console.log(
`depositFacet:: AFTER getDepositFacet(${derivedAddress})`,
depositFacet,
);
await Promise.all(
Object.values(payment).map(pmtP =>
E.when(pmtP, pmt => E(depositFacet).receive(pmt)),
),
);
// XXX partial failure? return payments?
// await Promise.all(
// E.when(
// payment,
// E(depositFacet).receive(payment),
// new Error('Error getting payment'),
// ),
// );
TT(
'After await E.when(payment, pmy => E(depositFacet).recieve(pmt)',
);

rearrange(
harden([
[tokenHolderSeat, claimSeat, { Tokens: paymentAmount }],
[claimSeat, tokenHolderSeat, { Fee: claimTokensFee }],
]),
harden([[claimSeat, tokenHolderSeat, { Fee: claimTokensFee }]]),
);

claimSeat.exit();

accountStore.add(pubkey, {
address,
address: derivedAddress,
pubkey,
tier,
amountAllocated: paymentAmount,
amountAllocated: payment.value,
epoch: this.state.currentEpoch,
});

Expand Down
5 changes: 4 additions & 1 deletion contract/src/airdrop.local.proposal.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Fail } from '@endo/errors';
import { makeMarshal } from '@endo/marshal';
import { makeTracer } from '@agoric/internal';
import { installContract } from './platform-goals/start-contract.js';
import { fixHub } from './fixHub.js';
import './types.js';

const contractName = 'tribblesAirdrop';
Expand Down Expand Up @@ -111,7 +112,7 @@ export const startAirdrop = async (powers, config) => {
const {
consume: {
namesByAddressAdmin,
namesByAddress,
// namesByAddress,
// bankManager,
board,
chainTimerService,
Expand Down Expand Up @@ -152,6 +153,7 @@ export const startAirdrop = async (powers, config) => {
customTerms?.merkleRoot,
'can not start contract without merkleRoot???',
);
const namesByAddress = await fixHub(namesByAddressAdmin);

const installation = await installContract(powers, {
name: contractName,
Expand All @@ -168,6 +170,7 @@ export const startAirdrop = async (powers, config) => {
issuerNames: ['Tribbles'],
privateArgs: harden({
timer,
namesByAddress,
}),
};
trace('BEFORE astartContract(permittedPowers, startOpts);', { startOpts });
Expand Down
12 changes: 6 additions & 6 deletions contract/src/airdrop.proposal.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { makeMarshal } from '@endo/marshal';
import { Fail } from '@endo/errors';
import { makeTracer, deeplyFulfilledObject } from '@agoric/internal';
import { makeStorageNodeChild } from '@agoric/internal/src/lib-chainStorage.js';
import { fixHub } from './fixHub.js';

const AIRDROP_TIERS_STATIC = [9000n, 6500n, 3500n, 1500n, 750n].map(
x => x * 1_000_000n,
Expand Down Expand Up @@ -121,7 +122,6 @@ export const startAirdrop = async (powers, config = defaultConfig) => {
const {
consume: {
namesByAddressAdmin,
namesByAddress,
bankManager,
board,
chainTimerService,
Expand Down Expand Up @@ -169,7 +169,7 @@ export const startAirdrop = async (powers, config = defaultConfig) => {
'can not start contract without merkleRoot???',
);
trace('AFTER assert(config?.options?.merkleRoot');
const marshaller = await E(board).getReadonlyMarshaller();
const namesByAddress = await fixHub(namesByAddressAdmin);

const startOpts = {
installation: await airdropInstallationP,
Expand All @@ -182,8 +182,7 @@ export const startAirdrop = async (powers, config = defaultConfig) => {
privateArgs: await deeplyFulfilledObject(
harden({
timer,
storageNode,
marshaller,
namesByAddress,
}),
),
};
Expand Down Expand Up @@ -297,14 +296,15 @@ export const permit = Object.values(airdropManifest)[0];
export const defaultProposalBuilder = async ({ publishRef, install }) => {
return harden({
// Somewhat unorthodox, source the exports from this builder module
sourceSpec: '@agoric/builders/scripts/testing/start-tribbles-airdrop.js',
sourceSpec:
'/workspaces/dapp-ertp-airdrop/contract/src/airdrop.proposal.js',
getManifestCall: [
'getManifestForAirdrop',
{
installKeys: {
tribblesAirdrop: publishRef(
install(
'@agoric/orchestration/src/examples/airdrop/airdrop.contract.js',
'/workspaces/dapp-ertp-airdrop/contract/src/airdrop.contract.js',
),
),
},
Expand Down
2 changes: 1 addition & 1 deletion contract/src/fixHub.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const { Fail } = assert;
/**
* ref https://github.com/Agoric/agoric-sdk/issues/8408#issuecomment-1741445458
*
* @param {ERef<import('@agoric/vats').NameAdmin>} namesByAddressAdmin
* @param {import('@agoric/vats').NameAdmin} namesByAddressAdmin
*/
export const fixHub = async namesByAddressAdmin => {
assert(namesByAddressAdmin, 'no namesByAddressAdmin???');
Expand Down
Loading
Loading