Update flow to include allowlist deploy and claiming #31
Conversation
Signed-off-by: Silas Davis <silas.davis@monaxlabs.io>
Signed-off-by: Silas Davis <silas.davis@monaxlabs.io>
Signed-off-by: Silas Davis <silas.davis@monaxlabs.io>
Also add support for: - Balance - ERC1155 detection Plus refactor partitioner logic into FeatureInterface Signed-off-by: Silas Davis <silas.davis@monaxlabs.io>
silasdavis
force-pushed
the
docs-allowlist
branch
from
8741acc
to
ef6d98f
Compare
| @@ -85,7 +80,7 @@ export class CollectionContract { | |||
| * @returns An object with mapped interface factories | |||
| */ | |||
| get interfaces(): Partial<FeatureInterfaces> { | |||
| if (!this._interfaces) throw InterfaceNotLoadedError; | |||
| if (!this._interfaces) throw new Error('Interface is not loaded'); | |||
There was a problem hiding this comment.
The main reason i've used InterfaceNotLoadedError is that being a const it can compared in catch blocks
There was a problem hiding this comment.
Yeah sorry, I get that, I meant to leave an explanation, I did actually leave a comment here and then just inlined it.
Comparable errors are definitely nice, but this method make debug stack traces next to useless because they stack is capture at definition time and show's you the module load trace rather than the actual error path.
See my comment here: https://github.com/monax/aspen-sdk/pull/31/files#r1073241898
Basically define a coded error with a fixed set of codes is my favoured approach - gives you comparability, preserves trace, and allows you to easily add additional context.
In platform we had some even more complicated 'chainable' errors which actually worked quite well but probably overkill here (linked in in-code comment)
(another example: https://github.com/hyperledger/burrow/blob/main/execution/errors/errors.go)
| constructor(base: CollectionContract) { | ||
| this.base = base; | ||
| // Late-bound because it must be called after load() | ||
| private getPartitioner = () => exhaustiveUnionPartitioner(this.base.interfaces, ...this.handledFeatures); |
There was a problem hiding this comment.
In my PR, memoisation is done on the base class and load() clears it, because it can be called multiple times.
There was a problem hiding this comment.
yes you are right.
I was also thinking that a call to getPartitioner may as well call load
Maybe we should make load implicit and reload explicit.
Of course this means that we need to agree that:
- All meaningful calls to features go via partitioner (not clear that's true)
- We don't mind make the partitioner call-path async (probably fine if the above is fine)
| makeGetPartition<P>( | ||
| partitionsThunk: (p: ReturnType<Features<T>['getPartitioner']>) => P, | ||
| ): <K extends keyof P>(k: K) => P[K] { | ||
| let t: P; |
There was a problem hiding this comment.
We should be able to bust the cache if load() is called again.
| export abstract class Features { | ||
| readonly base: CollectionContract; | ||
| export abstract class Features<T extends FeatureInterfaceId> { | ||
| protected constructor(readonly base: CollectionContract, readonly handledFeatures: readonly T[]) {} |
There was a problem hiding this comment.
Thank you!
Been trying yesterday (with no success) to make sure handledFeatures are typed so that we don't depend on people to proofread the interface names.
There was a problem hiding this comment.
I was golfing this a quite a bit, I wanted to move the definition of getPartitions up to base class but it was a bridge too far.
| @@ -628,6 +643,8 @@ export class Issuance extends Features { | |||
|
|
|||
| // this internal try catches the revert error | |||
| // which is a signal that the claim conditions aren't met | |||
| // FIXME[Silas]: This is throwing away the custom errors. They should be decoded and returned using `decodeCustomError` | |||
There was a problem hiding this comment.
Can we have a chat about the general tactics around errors. So far the approach has been that we don't throw any errors and don't return them. Obviously this will have to change.
There was a problem hiding this comment.
Shall we chat on our 1-1 later about this?
| // FIXME[Silas]: global constant errors break stack traces. Subclass Error with a string constant code instead. | ||
| // Also makes it easier to add error context. Possible inspiration: https://github.com/monax/platform/blob/main/pkg/api/src/services/errors.ts | ||
| // I have inlined this for now, leaving original definition here | ||
| // export const TokenIdRequiredError = new Error('Token is required for ERC1155 contracts!'); |
Signed-off-by: Silas Davis silas.davis@monaxlabs.io