Cache contract wrappers #334
Conversation
config/default.json
Outdated
| @@ -1,6 +1,7 @@ | |||
| { | |||
| "autoApproveTokenTransfers": true, | |||
| "defaultVotingMachine": "AbsoluteVote", | |||
| "cacheContractWrappers": false, | |||
There was a problem hiding this comment.
why not true? seems like a no-brainer to me
There was a problem hiding this comment.
Just being conservative: This feature consumes a technically limitless amount of memory. Maybe you're right though?
There was a problem hiding this comment.
hmm, only as much memory as there are contracts to cache
There was a problem hiding this comment.
yeah, that's why I said "technically"
| @@ -8,6 +10,18 @@ import { Web3EventService } from "./web3EventService"; | |||
| export class ContractWrapperFactory<TWrapper extends IContractWrapperBase> | |||
| implements IContractWrapperFactory<TWrapper> { | |||
|
|
|||
| public static setConfigService(configService: IConfigService): void { | |||
There was a problem hiding this comment.
in general would like a description of all the changes that are happening to the config service in the PR
There was a problem hiding this comment.
I did describe the change in the description
There was a problem hiding this comment.
... there is otherwise no change noticible to users of arc.js.
There was a problem hiding this comment.
But yes, I did make an internal change and could've mentioned that here
| @@ -49,9 +88,50 @@ export class ContractWrapperFactory<TWrapper extends IContractWrapperBase> | |||
| */ | |||
| public async deployed(): Promise<TWrapper> { | |||
| await this.ensureSolidityContract(); | |||
| return new this.wrapper(this.solidityContract, this.web3EventService).hydrateFromDeployed(); | |||
|
|
|||
| const getWrapper = (): Promise<TWrapper> => { | |||
There was a problem hiding this comment.
almost all this code is a duplicate of .at, any easy way to merge/reuse?
There was a problem hiding this comment.
I initially tried that but they are each a bit different. I decided it wasn't worth it.
There was a problem hiding this comment.
In the end it was possible
lib/contractWrapperFactory.ts
Outdated
| } | ||
| addressMap.set(wrapper.address, wrapper); | ||
| if (!at) { | ||
| // so we can also get the deployed version of this contract without the address |
There was a problem hiding this comment.
why not just use the deployed address as the name? otherwise you could have two copies of the same contract in the cache depending on if you call .deployed or .at
There was a problem hiding this comment.
The "${name}_deployed" key is for when users call deployed(), which supplies no address, only the name.
There was a problem hiding this comment.
right, but the deployed contract has an address, which you could use as the name of the key so that you dont end up with two copies of the same cached contract
There was a problem hiding this comment.
Found a way to do it
dkent600
force-pushed
the
cacheWrappers
branch
from
faaf710
to
bedc026
Compare
dkent600
force-pushed
the
cacheWrappers
branch
from
bedc026
to
f37aed7
Compare
| } | ||
|
|
||
| if (address) { | ||
| hydratedWrapper = this.getCachedContract(this.solidityContractName, address) as TWrapper; |
There was a problem hiding this comment.
should maybe log if the returned contract is coming from the cache or not?
There was a problem hiding this comment.
yeah, I was wanting to do that too. done.
Resolves: #324
Cache contract wrappers obtained using the contract wrapper factory methods
.atand.new. The cache is local, it does not persist across application instances.The feature is controlled by the
cacheContractWrappersconfig setting which is set totrueby default.