Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

Multi Token Pool #621

Merged
merged 1 commit into from
Jul 16, 2021
Merged

Multi Token Pool #621

merged 1 commit into from
Jul 16, 2021

Conversation

jacque006
Copy link
Collaborator

@jacque006 jacque006 commented Jul 8, 2021

Resolves #626

TODO

@github-actions github-actions bot added client This PR is about implementing the client contracts This PR changes some contracts labels Jul 8, 2021
@jacque006 jacque006 force-pushed the feature/multi-token-pool-party branch 3 times, most recently from e272486 to c673d7a Compare July 14, 2021 16:33
@@ -80,10 +80,11 @@ async function main() {
`Registering ${numPubkeys} pubkeys. Custom mnemonic: ${!!pubkeyMnemonic}`
);
const group = Group.new({
n: pubkeyMnemonic,
n: parseInt(numPubkeys),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pretty sure this derp was me, should work properly now.

@@ -116,7 +116,7 @@ export async function handleNewBatch(
const receipt = await event.getTransactionReceipt();
const logs = receipt.logs.map(log => rollup.interface.parseLog(log));
const depositsFinalisedLog = logs.filter(
log => log.signature == "DepositsFinalised(uint256,bytes32,uint256)"
log => log.signature === "DepositsFinalised(uint256,bytes32,uint256)"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -10,6 +10,8 @@ export interface Entry<Item> {
item: Item;
}

// TODO Update this interface and implementations to
// use BigNumber for itemID.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Avoided this refactor as this PR is already large and didn't want to bork @kautukkundan 's work :) BigNumberish should be fine as well as long as the storage engine and tree both store in a large value format under the covers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh makes sense, can discuss this

@@ -176,7 +176,7 @@ export function parseG2(solG2: solG2): mclG2 {
}

function dump(sol: solG1 | solG2): string {
return `0x${sol.map(n => n.toString().slice(2)).join()}`;
return `0x${sol.map(n => n.toString().slice(2)).join("")}`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is also my derp. join by default adds a comma separator which the previous implementation did not.

@jacque006 jacque006 marked this pull request as ready for review July 14, 2021 16:49
@jacque006
Copy link
Collaborator Author

@kautukkundan @ChihChengLiang your 👀 would be especially appreciated on number -> BigNumber refactor points. I avoided updating code in commitments.ts and StorageEngine, mainly focused on values in ts/features/transfers.

contracts/test/CustomToken.sol Outdated Show resolved Hide resolved
contracts/test/ExampleToken.sol Outdated Show resolved Hide resolved
// Setup and register custom tokens
const customTokens = await Promise.all([
new CustomToken__factory(signer).deploy("Hubble", "HUB"),
new CustomToken__factory(signer).deploy("Telescope", "TLSC")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Telescope token ❤️

@@ -13,6 +14,22 @@ import {

chai.use(chaiAsPromised);

const txFactory = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍🏼

@@ -10,6 +10,8 @@ export interface Entry<Item> {
item: Item;
}

// TODO Update this interface and implementations to
// use BigNumber for itemID.
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh makes sense, can discuss this

Copy link
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

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

Looks good to me

contracts/test/ExampleToken.sol Outdated Show resolved Hide resolved
ts/client/config.ts Show resolved Hide resolved
test/fast/txPool.test.ts Outdated Show resolved Hide resolved
ts/client/txPool.ts Show resolved Hide resolved
@jacque006 jacque006 force-pushed the feature/multi-token-pool-party branch from c673d7a to 5bc9dcb Compare July 16, 2021 14:59
SameTokenPool -> MultiTokenPool to support multiple token transactions.
Convert numerous number types to BigNumber to reflect L1 values and prevent overflows.
Add maxPendingTransactions to config file.
ExampleToken -> CustomToken test contract to aid in testing multiple ERC20 tokens.
Fix deploy script not generating the correct number of public keys.
Update client integration test and utilities to support multiple tokens.
Fix bug in mcl.dump where comma was being added to joined hexes.
@jacque006 jacque006 force-pushed the feature/multi-token-pool-party branch from 5bc9dcb to 0893844 Compare July 16, 2021 16:44
@jacque006 jacque006 merged commit 8de7ea9 into master Jul 16, 2021
@jacque006 jacque006 deleted the feature/multi-token-pool-party branch July 16, 2021 17:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
client This PR is about implementing the client contracts This PR changes some contracts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multi token transaction pool
3 participants